Dev #17350: Question theme should be a separate database column, not …

…an attribute (#1933) * Dev #17350: Question theme should be a separate database column, not an attribute * Dev #17350: Question theme should be a separate database column, not an attribute - Fix update process - Add relation to Question model * Dev #17350: Question theme should be a separate database column, not an attribute - Fixed 'createFieldMap' * Dev #17350: Question theme should be a separate database column, not an attribute - Updated QuestionAttribute::getQuestionTemplateValue() - Stop using QuestionAttribute::getQuestionTemplateValue() on QuestionAdministrationController - Added 'core()' scope to QuestionTheme (to filter only core/base themes) * Dev #17350: Question theme should be a separate database column, not an attribute - Updated QuestionAttribute::addAdditionalAttributesFromExtendedTheme() * Dev #17350: Question theme should be a separate database column, not an attribute - Updated QuestionTemplate::getQuestionTemplateFolderName() * Dev #17350: Question theme should be a separate database column, not an attribute - Update QuestionTheme::uninstall() * Dev #17350: Question theme should be a separate database column, not an attribute - Updated QuestionAttributeHelper * Dev #17350: Question theme should be a separate database column, not an attribute - Fix question create/edit - Fix update process to include user settings - Stop using "core" as question theme name (use the real name) * Dev #17350: Question theme should be a separate database column, not an attribute - Fixed formatting on updatedb_helper * Dev #17350: Question theme should be a separate database column, not an attribute - Deprecated QuestionAttribute::getQuestionTemplateValue() - Fixed message from QuestionTheme::uninstall() mixing 'question type' and 'question theme' * Dev #17350: Question theme should be a separate database column, not an attribute - Don't throw exception on findQuestionMetaData() when question theme name is empty * Dev #17350: Question theme should be a separate database column, not an attribute - Remove 'core' default value for question_theme_name on several functions - Rename scope for base themes - Replace question_theme relation on Question model for a getQuestionTheme() method - Change how findQuestionMetaData handles the theme base themes * Dev #17350: Question theme should be a separate database column, not an attribute - Fix wrong test * Dev #17350: Question theme should be a separate database column, not an attribute - Keep the question theme 'cached' on Question model * Dev #17350: Question theme should be a separate database column, not an attribute - Replace usage of findQuestionMetaData() on 'create.php' view in favor of ->getQuestionTheme() * Dev #17350: Question theme should be a separate database column, not an attribute - Revert commit 3ad22b5 which included wrong files * Dev #17350: Question theme should be a separate database column, not an attribute - Fix wrong test * Dev #17350: Question theme should be a separate database column, not an attribute - Add test for Question::getQuestionTheme() * Dev #17350: Question theme should be a separate database column, not an attribute - Fix error on QuestionCreate * Dev #17350: Question theme should be a separate database column, not an attribute - Use getRelated() on Question::getQuestionTheme() - Fix update procedure - Fix QuestionTheme::getDecodedSettings() * Dev #17350: Question theme should be a separate database column, not an attribute - Remove test for retrieving the proper question theme when no theme name is set * Dev #17350: Question theme should be a separate database column, not an attribute - Try to fix the tests * Dev #17350: Question theme should be a separate database column, not an attribute - Try to fix the tests Co-authored-by: encuestabizdevgit <devgit@encuesta.biz>
LimeSurvey · Aug 5, 2021 · c56c388 · c56c388
1 parent 3b82b14
commit c56c388
Show file tree

Hide file tree

Showing 22 changed files with 318 additions and 146 deletions.
diff --git a/application/config/config-defaults.php b/application/config/config-defaults.php
@@ -763,7 +763,7 @@
 
 // Preselected Question Type
 $config['preselectquestiontype'] = 'T';
-$config['preselectquestiontheme'] = 'core';
+$config['preselectquestiontheme'] = 'longfreetext';
 
 // theme editor mode
 $config['defaultthemeteeditormode'] = 'default';

diff --git a/application/config/version.php b/application/config/version.php
@@ -12,7 +12,7 @@
  */
 
 $config['versionnumber'] = '5.1.0-dev';
-$config['dbversionnumber'] = 460;
+$config['dbversionnumber'] = 470;
 $config['buildnumber'] = '';
 $config['updatable'] = true;
 $config['templateapiversion']  = 3;

diff --git a/application/controllers/QuestionAdministrationController.php b/application/controllers/QuestionAdministrationController.php
@@ -90,7 +90,7 @@ public function actionCreate($surveyid)
             throw new Exception('Internal error: Found no survey with id ' . $surveyid);
         }
 
-        $oQuestion = $this->getQuestionObject(null, null, null);
+        $oQuestion = $this->getQuestionObject();
         $oQuestion->sid = $surveyid;
 
         $this->aData['showSaveAndNewGroupButton'] = true;
@@ -178,17 +178,6 @@ public function renderFormAux(Question $question)
         App()->session['FileManagerContext'] = "edit:survey:{$question->sid}";
         initKcfinder();
 
-        $questionTemplate = SettingsUser::getUserSettingValue(
-            'preselectquestiontheme',
-            null,
-            null,
-            null,
-            App()->getConfig('preselectquestiontheme')
-        );
-        if ($question->qid !== 0) {
-            $questionTemplate = QuestionAttribute::getQuestionTemplateValue($question->qid);
-        }
-
         $this->aData['surveyid'] = $question->sid;
         $this->aData['sid'] = $question->sid;
         $this->aData['display']['menu_bars']['gid_action'] = 'viewquestion';
@@ -198,7 +187,7 @@ public function renderFormAux(Question $question)
             $question->survey->currentLanguageSettings->surveyls_title
             . " (" . gT("ID") . ":" . $question->sid . ")";
         $this->aData['aQuestionTypeList'] = QuestionTheme::findAllQuestionMetaDataForSelector();
-        $advancedSettings = $this->getAdvancedOptions($question->qid, $question->type, $questionTemplate);
+        $advancedSettings = $this->getAdvancedOptions($question->qid, $question->type, $question->question_theme_name);
         // Remove general settings from this array.
         unset($advancedSettings['Attribute']);
 
@@ -244,8 +233,7 @@ public function renderFormAux(Question $question)
             $question->qid,
             $question->type,
             $question->gid,
-            // TODO: question_template
-            'core'
+            $question->question_theme_name
         );
 
         if (App()->session['questionselectormode'] !== 'default') {
@@ -257,7 +245,6 @@ public function renderFormAux(Question $question)
         $viewData = [
             'oSurvey'                => $question->survey,
             'oQuestion'              => $question,
-            'questionTemplate'       => $questionTemplate,
             'aQuestionTypeGroups'    => $this->getQuestionTypeGroups($this->aData['aQuestionTypeList']),
             'advancedSettings'       => $advancedSettings,
             'generalSettings'        => $generalSettings,
@@ -565,7 +552,7 @@ public function actionSaveQuestionData()
      * @param string $sQuestionType
      * @param int $gid
      * @param boolean $returnArray
-     * @param string $question_template
+     * @param string $questionThemeName
      *
      * @return void|array
      * @throws CException
@@ -575,9 +562,9 @@ public function actionGetGeneralOptions(
         $sQuestionType = null,
         $gid = null,
         $returnArray = false,  //todo see were this ajaxrequest is done and take out the parameter there and here
-        $question_template = 'core'
+        $questionThemeName = null
     ) {
-        $aGeneralOptionsArray = $this->getGeneralOptions($iQuestionId, $sQuestionType, $gid, $question_template);
+        $aGeneralOptionsArray = $this->getGeneralOptions($iQuestionId, $sQuestionType, $gid, $questionThemeName);
 
         $this->renderJSON($aGeneralOptionsArray);
     }
@@ -590,7 +577,7 @@ public function actionGetGeneralOptions(
      * @param int $iQuestionId
      * @param string $sQuestionType
      * @param boolean $returnArray
-     * @param string $question_template
+     * @param string $questionThemeName
      *
      * @return void|array
      * @throws CException
@@ -600,11 +587,12 @@ public function actionGetAdvancedOptions(
         $iQuestionId = null,
         $sQuestionType = null,
         $returnArray = false, //todo see were this ajaxrequest is done and take out the parameter there and here
-        $question_template = 'core'
+        $questionThemeName = null
     ) {
         //here we get a Question object (also if question is new --> QuestionCreate)
-        $oQuestion = $this->getQuestionObject($iQuestionId, $sQuestionType);
-        $aAdvancedOptionsArray = $this->getAdvancedOptions($iQuestionId, $sQuestionType, $question_template);
+        // TODO: this object doesn't seem to be needed here.
+        $oQuestion = $this->getQuestionObject($iQuestionId, $sQuestionType, null, $questionThemeName);
+        $aAdvancedOptionsArray = $this->getAdvancedOptions($iQuestionId, $sQuestionType, $questionThemeName);
 
         $this->renderJSON(
             [
@@ -1542,7 +1530,7 @@ public function actionRenderItemsSelected()
      * @param int $questionId Null or 0 if new question is being created.
      * @return void
      */
-    public function actionGetGeneralSettingsHTML(int $surveyId, string $questionType, string $questionTheme = 'core', $questionId = null)
+    public function actionGetGeneralSettingsHTML(int $surveyId, string $questionType, string $questionTheme = null, $questionId = null)
     {
         if (empty($questionType)) {
             throw new CHttpException(405, 'Internal error: No question type');
@@ -1552,7 +1540,7 @@ public function actionGetGeneralSettingsHTML(int $surveyId, string $questionType
             throw new CHttpException(403, gT('No permission'));
         }
         // NB: This works even when $questionId is null (get default question values).
-        $question = $this->getQuestionObject($questionId, $questionType);
+        $question = $this->getQuestionObject($questionId, $questionType, null, $questionTheme);
         // NB: Only check permission when there is a question.
         if (!empty($question)) {
             // NB: Could happen if user manipulates request.
@@ -1743,7 +1731,7 @@ public function actionCopyQuestion()
      * @param int $questionId Null or 0 if new question is being created.
      * @return void
      */
-    public function actionGetAdvancedSettingsHTML(int $surveyId, string $questionType, string $questionTheme = 'core', $questionId = null)
+    public function actionGetAdvancedSettingsHTML(int $surveyId, string $questionType, string $questionTheme = null, $questionId = null)
     {
         if (empty($questionType)) {
             throw new CHttpException(405, 'Internal error: No question type');
@@ -1754,7 +1742,7 @@ public function actionGetAdvancedSettingsHTML(int $surveyId, string $questionTyp
         }
         Yii::app()->loadHelper("admin.htmleditor");
         // NB: This works even when $questionId is null (get default question values).
-        $question = $this->getQuestionObject($questionId, $questionType);
+        $question = $this->getQuestionObject($questionId, $questionType, null, $questionTheme);
         if ($questionId) {
             // NB: Could happen if user manipulates request.
             if (!Permission::model()->hasSurveyPermission($question->sid, 'surveycontent', 'update')) {
@@ -2242,7 +2230,7 @@ public static function getDefaultValues(int $iSurveyID, int $gid, int $qid)
      * @return Question
      * @throws CException
      */
-    private function getQuestionObject($iQuestionId = null, $sQuestionType = null, $gid = null)
+    private function getQuestionObject($iQuestionId = null, $sQuestionType = null, $gid = null, $questionThemeName = null)
     {
         //todo: this should be done in the action directly
         $iSurveyId = App()->request->getParam('sid') ??
@@ -2252,13 +2240,17 @@ private function getQuestionObject($iQuestionId = null, $sQuestionType = null, $
         $oQuestion = Question::model()->findByPk($iQuestionId);
 
         if (empty($oQuestion)) {
-            $oQuestion = QuestionCreate::getInstance($iSurveyId, $sQuestionType);
+            $oQuestion = QuestionCreate::getInstance($iSurveyId, $sQuestionType, $questionThemeName);
         }
 
         if ($sQuestionType != null) {
             $oQuestion->type = $sQuestionType;
         }
 
+        if ($questionThemeName != null) {
+            $oQuestion->question_theme_name = $questionThemeName;
+        }
+
         if ($gid != null) {
             $oQuestion->gid = $gid;
         }
@@ -2272,7 +2264,7 @@ private function getQuestionObject($iQuestionId = null, $sQuestionType = null, $
      * @param int $iQuestionId
      * @param string $sQuestionType
      * @param int $gid
-     * @param string $question_template
+     * @param string $questionThemeName
      *
      * @return void|array
      * @throws CException
@@ -2281,12 +2273,12 @@ private function getGeneralOptions(
         $iQuestionId = null,
         $sQuestionType = null,
         $gid = null,
-        $question_template = 'core'
+        $questionThemeName = null
     ) {
-        $oQuestion = $this->getQuestionObject($iQuestionId, $sQuestionType, $gid);
+        $oQuestion = $this->getQuestionObject($iQuestionId, $sQuestionType, $gid, $questionThemeName);
         $result = $oQuestion
             ->getDataSetObject()
-            ->getGeneralSettingsArray($oQuestion->qid, $sQuestionType, null, $question_template);
+            ->getGeneralSettingsArray($oQuestion->qid, $sQuestionType, null, $questionThemeName);
         return $result;
     }
 
@@ -2366,13 +2358,13 @@ function ($oAnswerOption) {
      * @throws CException
      * @throws Exception
      */
-    private function getAdvancedOptions($iQuestionId = null, $sQuestionType = null, $sQuestionTheme = 'core')
+    private function getAdvancedOptions($iQuestionId = null, $sQuestionType = null, $sQuestionTheme = null)
     {
         //here we get a Question object (also if question is new --> QuestionCreate)
-        $oQuestion = $this->getQuestionObject($iQuestionId, $sQuestionType);
+        $oQuestion = $this->getQuestionObject($iQuestionId, $sQuestionType, null, $sQuestionTheme);
 
         // Get the advanced settings array
-        $advancedSettings = $oQuestion->getAdvancedSettingsWithValues(null, $sQuestionTheme);
+        $advancedSettings = $oQuestion->getAdvancedSettingsWithValues();
 
         // Group the array in categories
         $questionAttributeHelper = new LimeSurvey\Models\Services\QuestionAttributeHelper();
@@ -3162,17 +3154,12 @@ public function actionGetSummaryHTML(int $questionId)
             throw new CHttpException(403, gT('No permission'));
         }
 
-        /** @var string */
-        $questionThemeName = $question->getQuestionAttribute('question_template');
-
+        // Use the question's theme if it exists, or a dummy theme if it doesn't
         /** @var QuestionTheme */
-        $questionTheme = QuestionTheme::findQuestionMetaData($question->type, $questionThemeName);
-        if (empty($questionTheme['extends'])) {
-            $questionTheme['name'] = 'core';    // Temporary solution for the issue 17346
-        }
+        $questionTheme = !empty($question->questionTheme) ? $question->questionTheme : QuestionTheme::getDummyInstance($question->type);
 
         /** @var array<string,array<mixed>> */
-        $advancedSettings = $this->getAdvancedOptions($question->qid, $question->type, $questionThemeName);
+        $advancedSettings = $this->getAdvancedOptions($question->qid, $question->type, $question->question_theme_name);
         // Remove general settings from this array.
         unset($advancedSettings['Attribute']);
 

diff --git a/application/extensions/GeneralOptionWidget/settings/QuestionThemeGeneralOption.php b/application/extensions/GeneralOptionWidget/settings/QuestionThemeGeneralOption.php
@@ -34,7 +34,7 @@ public function __construct($currentSetQuestionTheme, array $options)
     public static function make(Question $question, $questionType, $currentSetQuestionTheme)
     {
         $aQuestionTemplateList = QuestionTemplate::getQuestionTemplateList($questionType);
-        $aQuestionTemplateAttributes = $question->getAdvancedSettingsWithValues()['question_template'];
+        $aQuestionTemplateAttributes = $question->question_theme_name;
 
         $aOptionsArray = [];
         foreach ($aQuestionTemplateList as $code => $value) {

diff --git a/application/helpers/admin/import_helper.php b/application/helpers/admin/import_helper.php
@@ -427,6 +427,18 @@ function XMLImportGroup($sFullFilePath, $iNewSID, $bTranslateLinksFields)
             }
             $insertdata['qid'] = $aQIDReplacements[(int) $insertdata['qid']]; // remap the parent_qid
 
+            // Question theme was previously stored as a question attribute ('question_template'), but now it
+            // is a normal attribute of the Question model. So we must check if the imported question has the
+            // 'question_template' attribute and use it for overriding 'question_theme_name' instead of saving
+            // as QuestionAttribute.
+            if ($insertdata['attribute'] == 'question_template') {
+                $oQuestion = Question::model()->findByPk($insertdata['qid']);
+                if (!empty($oQuestion)) {
+                    $oQuestion->question_theme_name = $insertdata['value'];
+                    $oQuestion->save();
+                }
+                continue;
+            }
 
             if (
                 $iDBVersion < 156 && isset($aAllAttributes[$insertdata['attribute']]['i18n']) &&
@@ -873,6 +885,19 @@ function XMLImportQuestion($sFullFilePath, $iNewSID, $iNewGID, $options = array(
                 $insertdata['qid'] = $aQIDReplacements[(int) $insertdata['qid']]; // remap the parent_qid
             }
 
+            // Question theme was previously stored as a question attribute ('question_template'), but now it
+            // is a normal attribute of the Question model. So we must check if the imported question has the
+            // 'question_template' attribute and use it for overriding 'question_theme_name' instead of saving
+            // as QuestionAttribute.
+            if ($insertdata['attribute'] == 'question_template') {
+                $oQuestion = Question::model()->findByPk($insertdata['qid']);
+                if (!empty($oQuestion)) {
+                    $oQuestion->question_theme_name = $insertdata['value'];
+                    $oQuestion->save();
+                }
+                continue;
+            }
+
             if (
                 $iDBVersion < 156 &&
                 isset($aAllAttributes[$insertdata['attribute']]['i18n']) &&
@@ -1732,6 +1757,20 @@ function XMLImportSurvey($sFullFilePath, $sXMLdata = null, $sNewSurveyName = nul
             }
 
             $insertdata['qid'] = $aQIDReplacements[(int) $insertdata['qid']]; // remap the qid
+
+            // Question theme was previously stored as a question attribute ('question_template'), but now it
+            // is a normal attribute of the Question model. So we must check if the imported question has the
+            // 'question_template' attribute and use it for overriding 'question_theme_name' instead of saving
+            // as QuestionAttribute.
+            if ($insertdata['attribute'] == 'question_template') {
+                $oQuestion = Question::model()->findByPk($insertdata['qid']);
+                if (!empty($oQuestion)) {
+                    $oQuestion->question_theme_name = $insertdata['value'];
+                    $oQuestion->save();
+                }
+                continue;
+            }
+
             if ($iDBVersion < 156 && isset($aAllAttributes[$insertdata['attribute']]['i18n']) && $aAllAttributes[$insertdata['attribute']]['i18n']) {
                 foreach ($aLanguagesSupported as $sLanguage) {
                     $insertdata['language'] = $sLanguage;

diff --git a/application/helpers/common_helper.php b/application/helpers/common_helper.php
@@ -1434,12 +1434,11 @@ function createFieldMap($survey, $style = 'short', $force_refresh = false, $ques
 
     // Main query
     $quotedGroups = Yii::app()->db->quoteTableName('{{groups}}');
-    $aquery = "SELECT g.*, q.*, gls.*, qls.*, qa.attribute, qa.value"
+    $aquery = "SELECT g.*, q.*, gls.*, qls.*"
     . " FROM $quotedGroups g"
     . ' JOIN {{questions}} q on q.gid=g.gid '
     . ' JOIN {{group_l10ns}} gls on gls.gid=g.gid '
     . ' JOIN {{question_l10ns}} qls on qls.qid=q.qid '
-    . " LEFT JOIN {{question_attributes}} qa ON qa.qid=q.qid AND qa.attribute='question_template' "
     . " WHERE qls.language='{$sLanguage}' and gls.language='{$sLanguage}' AND"
     . " g.sid={$surveyid} AND"
     . " q.parent_qid=0";
@@ -1469,8 +1468,8 @@ function createFieldMap($survey, $style = 'short', $force_refresh = false, $ques
         $usedinconditions = 'N';
 
         // Check if answertable has custom setting for current question
-        if (isset($arow['attribute']) && isset($arow['type']) && $arow['attribute'] == 'question_template') {
-            $answerColumnDefinition = QuestionTheme::getAnswerColumnDefinition($arow['value'], $arow['type']);
+        if (isset($arow['attribute']) && isset($arow['type']) && isset($arow['question_theme_name'])) {
+            $answerColumnDefinition = QuestionTheme::getAnswerColumnDefinition($arow['question_theme_name'], $arow['type']);
         }
 
         // Field identifier

diff --git a/application/helpers/update/updatedb_helper.php b/application/helpers/update/updatedb_helper.php
@@ -4744,6 +4744,31 @@ function ($v) {
             $oDB->createCommand()->update('{{settings_global}}', ['stg_value' => 460], "stg_name='DBVersion'");
             $oTransaction->commit();
         }
+
+        if ($iOldDBVersion < 470) {
+            $oTransaction = $oDB->beginTransaction();
+            // Add the new column to questions table
+            $oDB->createCommand()->addColumn('{{questions}}', 'question_theme_name', 'string(150) NULL');
+            // Fill column from question_attributes when it's not null or 'core'
+            $oDB->createCommand("UPDATE {{questions}} q LEFT JOIN {{question_attributes}} qt ON qt.qid = q.qid AND qt.attribute = 'question_template'
+                 SET q.question_theme_name = qt.value 
+                 WHERE qt.value IS NOT NULL AND qt.value <> 'core' AND qt.value <> ''")->execute();
+            // Fill null question_theme_name values using the proper theme name
+            $oDB->createCommand("UPDATE {{questions}} q LEFT JOIN {{question_themes}} qt ON qt.question_type = q.type AND qt.core_theme = 1 AND qt.extends = ''
+                 SET q.question_theme_name = qt.name 
+                 WHERE q.question_theme_name IS NULL")
+            ->execute();
+
+            // Also update 'preselectquestiontheme' user settings where the value is 'core'
+            $oDB->createCommand("UPDATE {{settings_user}} su
+                JOIN {{settings_user}} su2 ON su2.uid = su.uid AND su2.stg_name = 'preselectquestiontype'
+                JOIN {{question_themes}} qt ON qt.question_type = su2.stg_value
+                SET su.stg_value = qt.name
+                WHERE su.stg_name = 'preselectquestiontheme' AND su.stg_value = 'core'")->execute();
+
+            $oDB->createCommand()->update('{{settings_global}}', array('stg_value' => 470), "stg_name='DBVersion'");
+            $oTransaction->commit();
+        }
     } catch (Exception $e) {
         Yii::app()->setConfig('Updating', false);
         $oTransaction->rollback();