From c56c388e37f5614916af4da06d02c09eb7a926d0 Mon Sep 17 00:00:00 2001 From: Gabriel Jenik Date: Thu, 5 Aug 2021 18:08:29 -0300 Subject: [PATCH 001/170] 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 3ad22b5d0665120cdd932e8ab09a646343cd85b0 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 --- application/config/config-defaults.php | 2 +- application/config/version.php | 2 +- .../QuestionAdministrationController.php | 75 ++++------ .../settings/QuestionThemeGeneralOption.php | 2 +- application/helpers/admin/import_helper.php | 39 +++++ application/helpers/common_helper.php | 7 +- .../helpers/update/updatedb_helper.php | 25 ++++ application/models/Question.php | 36 ++++- application/models/QuestionAttribute.php | 26 ++-- application/models/QuestionBaseDataSet.php | 4 +- application/models/QuestionCreate.php | 11 +- application/models/QuestionTemplate.php | 6 +- application/models/QuestionTheme.php | 141 ++++++++++++------ .../ThemeQuestionAttributeProvider.php | 2 +- .../views/questionAdministration/create.php | 11 +- .../views/questionAdministration/summary.php | 8 +- .../questionAdministration/typeSelector.php | 15 +- assets/scripts/admin/questionEditor.js | 2 +- installer/create-database.php | 1 + .../backend/QuestionAttributeTest.php | 2 +- .../backend/QuestionGroupEditorTest.php | 2 + tests/unit/models/QuestionTest.php | 45 ++++++ 22 files changed, 318 insertions(+), 146 deletions(-) create mode 100644 tests/unit/models/QuestionTest.php diff --git a/application/config/config-defaults.php b/application/config/config-defaults.php index 2ce7050623d..9c2469a33ea 100644 --- 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 index 899b293d4ea..ce057bc7828 100644 --- 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 index 11307d13137..cf69341488f 100644 --- 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> */ - $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 index 58c53a62266..2cc18b9c393 100644 --- 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 index 2734ceb3b32..46474f33ae9 100644 --- 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 index 72634597088..9304cd50332 100644 --- 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 index 5ecbf6d6347..43e499b0847 100644 --- 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(); diff --git a/application/models/Question.php b/application/models/Question.php index 10e435aca1a..e26c4675200 100644 --- a/application/models/Question.php +++ b/application/models/Question.php @@ -89,6 +89,9 @@ class Question extends LSActiveRecord /** Set defaut relevance **/ public $relevance =''; + /** @var QuestionTheme cached question theme*/ + private $relatedQuestionTheme; + /** * @inheritdoc * @return Question @@ -124,6 +127,9 @@ public function relations() 'subquestions' => array(self::HAS_MANY, 'Question', array('parent_qid' => 'qid'), 'order' => App()->getDb()->quoteColumnName('subquestions.question_order') . ' ASC'), 'conditions' => array(self::HAS_MANY, 'Condition', 'qid'), 'answers' => array(self::HAS_MANY, 'Answer', 'qid', 'order' => App()->getDb()->quoteColumnName('answers.sortorder') . ' ASC'), + // This relation will fail for non saved questions, which is often the case + // when using question editor on create mode. Better use getQuestionTheme() + 'question_theme' => [self::HAS_ONE, 'QuestionTheme', ['question_type' => 'type', 'name' => 'question_theme_name']], ); } @@ -214,6 +220,8 @@ public function rules() 'message' => sprintf(gT("Code: '%s' is a reserved word."), $this->title), // Usage of {attribute} need attributeLabels, {value} never exist in message 'except' => 'archiveimport' ); + /* Don't save empty or 'core' question theme name */ + $aRules[] = ['question_theme_name', 'questionThemeNameValidator']; } else { $aRules[] = array('title', 'compare', 'compareValue' => 'time', 'operator' => '!=', 'message' => gT("'time' is a reserved word and can not be used for a subquestion."), @@ -1535,13 +1543,31 @@ public function getScaledSubquestions() } /** - * Get the question theme name + * Validates the question theme name, making sure it's not empty or 'core' + */ + public function questionThemeNameValidator() + { + // As long as there is a question theme name, and it's not 'core', it's ok. + if (!empty($this->question_theme_name) && $this->question_theme_name != 'core') { + return; + } + + // If question_theme_name is empty or 'core', we fetch the value from the question_theme related to the question_type + $baseQuestionThemeName = QuestionTheme::model()->getBaseThemeNameForQuestionType($this->type); + if (!empty($baseQuestionThemeName)) { + $this->question_theme_name = $baseQuestionThemeName; + } + } + + /** + * Returns the QuestionTheme related to this question. + * It's not implemented as a relation because relations only work on + * persisted models. * - * @return string + * @return QuestionTheme|null */ - public function getQuestionThemeName() + public function getQuestionTheme() { - $questionTheme = $this->getQuestionAttribute('question_template'); - return !empty($questionTheme) ? $questionTheme : self::DEFAULT_QUESTION_THEME; + return $this->getRelated("question_theme", $this->isNewRecord); } } diff --git a/application/models/QuestionAttribute.php b/application/models/QuestionAttribute.php index f465cb5b1be..be37e61883c 100644 --- a/application/models/QuestionAttribute.php +++ b/application/models/QuestionAttribute.php @@ -292,14 +292,10 @@ public static function getAttributesAsArrayFromDB($iQuestionID) public static function addAdditionalAttributesFromExtendedTheme($aAttributeNames, $oQuestion) { $retAttributeNamesExtended = $aAttributeNames; - /* @var $oAttributeValue QuestionAttribute*/ - $oAttributeValue = self::model()->resetScope()->find( - "qid=:qid and attribute=:attribute", - ['qid' => $oQuestion->qid, 'attribute' => 'question_template'] - ); - if ($oAttributeValue !== null) { - $sQuestionTheme = $oAttributeValue->value; - $aThemeAttributes = QuestionTheme::getAdditionalAttrFromExtendedTheme($sQuestionTheme, $oQuestion->type); + /** @var string|null */ + $questionThemeName = $oQuestion->question_theme_name; + if (!empty($questionThemeName)) { + $aThemeAttributes = QuestionTheme::getAdditionalAttrFromExtendedTheme($questionThemeName, $oQuestion->type); $questionAttributeHelper = new QuestionAttributeHelper(); $retAttributeNamesExtended = $questionAttributeHelper->mergeQuestionAttributes($retAttributeNamesExtended, $aThemeAttributes); } @@ -428,15 +424,17 @@ public static function getQuestionAttributesSettings($sType, $advancedOnly = fal * the attribute is missing. In those cases, the deault "core" is used. * * @return string question_template or 'core' if it not exists + * + * @deprecated use $question->question_theme_name instead (Question model) */ public static function getQuestionTemplateValue($questionID) { - $question_template = QuestionAttribute::model()->findByAttributes([ - 'qid' => $questionID, - 'attribute' => 'question_template' - ]); - - $value = !empty($question_template) && !empty($question_template->value) ? $question_template->value : 'core'; + /** + * TODO: This method was modified to get the theme name from the proper place, but it should be deprecated, + * as it no longer makes sense (question theme is not a QuestionAttribute anymore). + */ + $question = Question::model()->findByPk($questionID); + $value = !empty($question) && !empty($question->question_theme_name) ? $question->question_theme_name : 'core'; return $value; } diff --git a/application/models/QuestionBaseDataSet.php b/application/models/QuestionBaseDataSet.php index 755a6c1f0e5..c70349b5c60 100644 --- a/application/models/QuestionBaseDataSet.php +++ b/application/models/QuestionBaseDataSet.php @@ -27,12 +27,12 @@ public function __construct($iQuestionId) * @param int $iQuestionID * @param int $sQuestionType * @param string $sLanguage - * @param null $question_template + * @param null $questionThemeName * * @return array * @throws CException */ - public function getGeneralSettingsArray($iQuestionID = null, $sQuestionType = null, $sLanguage = null, $question_template = null) + public function getGeneralSettingsArray($iQuestionID = null, $sQuestionType = null, $sLanguage = null, $questionThemeName = null) { Yii::import('ext.GeneralOptionWidget.settings.*'); if ($iQuestionID != null) { diff --git a/application/models/QuestionCreate.php b/application/models/QuestionCreate.php index 33a7925f9bb..0a483822da3 100644 --- a/application/models/QuestionCreate.php +++ b/application/models/QuestionCreate.php @@ -8,7 +8,7 @@ class QuestionCreate extends Question /** * @todo This is a factory method, not a singleton. Rename to make() or create(). */ - public static function getInstance($iSurveyId, $type = null) + public static function getInstance($iSurveyId, $type = null, $themeName = null) { $oSurvey = Survey::model()->findByPk($iSurveyId); if (empty($oSurvey)) { @@ -23,6 +23,14 @@ public static function getInstance($iSurveyId, $type = null) } else { $questionType = SettingsUser::getUserSettingValue('preselectquestiontype', null, null, null, Yii::app()->getConfig('preselectquestiontype')); } + if (isset($themeName) && !empty($themeName)) { + $questionThemeName = $themeName; + } else { + $questionThemeName = SettingsUser::getUserSettingValue('preselectquestiontheme', null, null, null, Yii::app()->getConfig('preselectquestiontheme')); + } + if (empty($questionThemeName)) { + $questionThemeName = QuestionTheme::model()->getBaseThemeNameForQuestionType($questionType); + } $oCurrentGroup = QuestionGroup::model()->findByPk($gid); $temporaryTitle = 'G' . str_pad($oCurrentGroup->group_order, 2, '0', STR_PAD_LEFT) @@ -39,6 +47,7 @@ public static function getInstance($iSurveyId, $type = null) 'modulename' => '', 'title' => $temporaryTitle, 'question_order' => 9999, + 'question_theme_name' => $questionThemeName, ]; $oQuestion = new QuestionCreate(); diff --git a/application/models/QuestionTemplate.php b/application/models/QuestionTemplate.php index db0c6d2e05d..b6011fd3673 100644 --- a/application/models/QuestionTemplate.php +++ b/application/models/QuestionTemplate.php @@ -140,8 +140,10 @@ public function getTemplatePath() public function getQuestionTemplateFolderName() { if ($this->sTemplateFolderName === null) { - $aQuestionAttributes = QuestionAttribute::model()->getQuestionAttributes($this->oQuestion->qid); - $this->sTemplateFolderName = (isset($aQuestionAttributes['question_template']) && $aQuestionAttributes['question_template'] != 'core') ? $aQuestionAttributes['question_template'] : false; + $aQuestionAttributes = QuestionAttribute::model()->getQuestionAttributes($this->oQuestion->qid); + /** @var string|null */ + $questionThemeName = $this->oQuestion->question_theme_name; + $this->sTemplateFolderName = (!empty($questionThemeName) && $questionThemeName != 'core') ? $questionThemeName : false; } $this->bHasTemplate = ($this->sTemplateFolderName != false); return $this->sTemplateFolderName; diff --git a/application/models/QuestionTheme.php b/application/models/QuestionTheme.php index e9cc4f3fac2..198ed08e9c8 100644 --- a/application/models/QuestionTheme.php +++ b/application/models/QuestionTheme.php @@ -82,6 +82,18 @@ public function relations() return array(); } + /** @inheritdoc */ + public function scopes() + { + return array( + // 'base' themes are the ones that don't extend any question type/theme. + 'base' => array( + 'condition' => 'core_theme = :true AND extends = :extends', + 'params' => array(':true' => 1, ':extends' => '') + ), + ); + } + /** * @return array customized attribute labels (name=>label) */ @@ -504,34 +516,39 @@ public static function uninstall($oQuestionTheme) }; } - // transform theme name compatible with question attributes for core/default theme_template - $sThemeName = empty($oQuestionTheme->extends) ? 'core' : $oQuestionTheme->name; - // todo optimize function for very big surveys, eventually in yii 2 or 3 with batch processing / if this is breaking in Yii 1 use CDbDataReader $query = new CDbDataReader($command), $query->read() - $aQuestions = Question::model()->with('questionattributes')->findAll( - 'type = :type AND parent_qid = :parent_qid', + $aQuestions = Question::model()->findAll( + 'type = :type AND question_theme_name = :theme AND parent_qid = :parent_qid', [ ':type' => $oQuestionTheme->question_type, + ':theme' => $oQuestionTheme->name, ':parent_qid' => 0 ] ); - foreach ($aQuestions as $oQuestion) { - if (isset($oQuestion['questionattributes']['question_template'])) { - if ($sThemeName === $oQuestion['questionattributes']['question_template']['value']) { - $bDeleteTheme = false; - break; - } - } else { - if ($sThemeName === 'core') { - $bDeleteTheme = false; - break; - } + if (!empty($aQuestions)) { + // There are questions using this theme. Don't delete it + $bDeleteTheme = false; + } + + // Just in case, if this is a core (base) theme we also check if there are any questions without theme name (this shouldn't happen) + if (empty($oQuestionTheme->extends) && $bDeleteTheme !== false) { + $aQuestions = Question::model()->findAll( + "type = :type AND (question_theme_name = '' OR question_theme_name IS NULL) AND parent_qid = :parent_qid", + [ + ':type' => $oQuestionTheme->question_type, + ':parent_qid' => 0 + ] + ); + if (!empty($aQuestions)) { + // There are questions using this theme. Don't delete it + $bDeleteTheme = false; } } + // if this questiontheme is used, it cannot be deleted if (isset($bDeleteTheme) && !$bDeleteTheme) { return [ - 'error' => gT('Question type is used in a Survey and cannot be uninstalled'), + 'error' => gT('Question theme is used in a Survey and cannot be uninstalled'), 'result' => false ]; } @@ -574,44 +591,27 @@ public static function findQuestionMetaDataForAllTypes() * Returns all QuestionTheme settings * * @param string $question_type - * @param string $question_template 'core' OR question theme name + * @param string $question_theme_name * @param string $language * @return QuestionTheme */ - public static function findQuestionMetaData($question_type, $question_template = 'core', $language = '') + public static function findQuestionMetaData($question_type, $question_theme_name = null, $language = '') { if (empty($question_type)) { throw new InvalidArgumentException('question_type cannot be empty'); } - if (empty($question_template)) { - throw new InvalidArgumentException('question_template cannot be empty'); - } - - $criteria = new CDbCriteria(); - - if ($question_template === 'core') { - $criteria->condition = 'extends = :extends'; - $criteria->addCondition('question_type = :question_type', 'AND'); - $criteria->params = [':extends' => '', ':question_type' => $question_type]; + if (empty($question_theme_name) || $question_theme_name === 'core') { + $questionTheme = self::model()->base()->findByAttributes(['question_type' => $question_type]); } else { + $criteria = new CDbCriteria(); $criteria->addCondition('question_type = :question_type AND name = :name'); - $criteria->params = [':question_type' => $question_type, ':name' => $question_template]; + $criteria->params = [':question_type' => $question_type, ':name' => $question_theme_name]; + $questionTheme = self::model()->query($criteria, false); } - $questionTheme = self::model()->query($criteria, false); - if (empty($questionTheme)) { - $settings = new StdClass(); - $settings->class = ''; - $settings->answerscales = 0; - $settings->subquestions = 0; - $questionTheme = new self(); - $questionTheme->title = gT('Question theme error: Missing metadata'); - $questionTheme->name = gT('Question theme error: Missing metadata'); - $questionTheme->question_type = $question_type; - $questionTheme->settings = $settings; - return $questionTheme; + return self::getDummyInstance($question_type); } // language settings @@ -663,10 +663,6 @@ public static function findAllQuestionMetaDataForSelector() // decode settings json $baseQuestion['settings'] = json_decode($baseQuestion['settings']); - // if its a core question change name to core for rendering Default rendering in the selector - if (empty($baseQuestion['extends'])) { - $baseQuestion['name'] = 'core'; - } $baseQuestion['image_path'] = str_replace( '//', '/', @@ -774,8 +770,8 @@ public static function getAnswerColumnDefinition($name, $type) return $cacheMemo[$cacheKey]; } - if ($name == 'core') { - $questionTheme = self::model()->findByAttributes([], 'question_type=:question_type AND extends=:extends', ['question_type' => $type, 'extends' => '']); + if (empty($name) || $name == 'core') { + $questionTheme = self::model()->base()->findByAttributes(['question_type' => $type, 'extends' => '']); } else { $questionTheme = self::model()->findByAttributes([], 'name=:name AND question_type=:question_type', ['name' => $name, 'question_type' => $type]); } @@ -1030,4 +1026,53 @@ public static function getThemeDirectoryPath($sQuestionConfigFilePath) } return $sThemeDirectoryName; } + + /** + * Returns the name of the base question theme for the question type $questionType + * + * @param string $questionType + * @return string|null question theme name or null if no question theme is found + */ + public function getBaseThemeNameForQuestionType($questionType) + { + $questionTheme = $this->base()->findByAttributes(['question_type' => $questionType]); + if (!empty($questionTheme)) { + return $questionTheme->name; + } + } + + /** + * Returns the settings attribute decoded + * @return mixed + */ + public function getDecodedSettings() + { + if (is_object($this->settings)) { + return $this->settings; + } else { + return json_decode($this->settings); + } + } + + /** + * Returns a dummy instance of QuestionTheme, with + * the question type $questionType. + * + * @param string $questionType + * @return QuestionTheme + */ + public static function getDummyInstance($questionType) + { + $settings = new StdClass(); + $settings->class = ''; + $settings->answerscales = 0; + $settings->subquestions = 0; + + $questionTheme = new self(); + $questionTheme->title = gT('Question theme error: Missing metadata'); + $questionTheme->name = gT('Question theme error: Missing metadata'); + $questionTheme->question_type = $questionType; + $questionTheme->settings = $settings; + return $questionTheme; + } } diff --git a/application/models/services/ThemeQuestionAttributeProvider.php b/application/models/services/ThemeQuestionAttributeProvider.php index 5ff4a79264f..d0dc221c844 100644 --- a/application/models/services/ThemeQuestionAttributeProvider.php +++ b/application/models/services/ThemeQuestionAttributeProvider.php @@ -76,7 +76,7 @@ private static function getQuestionTheme($options) return $options['questionTheme']; } if (!empty($options['question'])) { - return $options['question']->questionThemeName; + return $options['question']->question_theme_name; } return ''; } diff --git a/application/views/questionAdministration/create.php b/application/views/questionAdministration/create.php index b21941c2058..2232b66c2cf 100644 --- a/application/views/questionAdministration/create.php +++ b/application/views/questionAdministration/create.php @@ -2,7 +2,6 @@ /** @var Survey $oSurvey */ /** @var Question $oQuestion */ -/** @var string $questionTemplate */ ?>