Skip to content

Commit

Permalink
Dev Fixed issue where a survey with duplicated question codes would n…
Browse files Browse the repository at this point in the history
…ot be

imported.
Dev Question codes must now start with a letter and contain only
alphanumeric characters.
  • Loading branch information
SamMousa committed Oct 21, 2013
1 parent d94b055 commit ae2a46c
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 1 deletion.
35 changes: 34 additions & 1 deletion application/helpers/admin/import_helper.php
Expand Up @@ -3679,7 +3679,40 @@ function XMLImportSurvey($sFullFilepath,$sXMLdata=NULL,$sNewSurveyName=NULL,$iDe
}
if ($insertdata)
XSSFilterArray($insertdata);
$newsqid =Question::model()->insertRecords($insertdata) or safeDie($clang->gT("Error").": Failed to insert data [5]<br />");
$question = new Question();
$question->setAttributes($insertdata, false);
$attempts = 0;
while (!$question->validate(array('title')))
{
if (is_numeric($question->title))
{
$question->title = 'q' . $question->title;
}
else
{
if (!isset($index))
{
$index = 0;
$rand = mt_rand(0, 1024);
}
else
{
$index++;
}
$question->title = 'r' . $rand . 'q' . $index;
}
$attempts++;
if ($attempts > 10)
{
safeDie($clang->gT("Error").": Failed to resolve question code problems after 10 attempts.<br />");
}
}
if (!$question->save())
{
safeDie($clang->gT("Error while saving: "). print_r($question->errors, true));
}
$newsqid = $question->qid;
//$newsqid =Question::model()->insertRecords($insertdata) or safeDie($clang->gT("Error").": Failed to insert data [5]<br />");
if (!isset($insertdata['qid']))
{
$aQIDReplacements[$oldsqid]=$newsqid; // add old and new qid to the mapping array
Expand Down
9 changes: 9 additions & 0 deletions application/models/Question.php
Expand Up @@ -88,6 +88,15 @@ public function rules()
'message'=>'{attribute} "{value}" is already in use.'),
array('language','length', 'min' => 2, 'max'=>20),// in array languages ?
array('title,question,help','LSYii_Validators'),
array('title', 'unique', 'caseSensitive'=>true, 'criteria'=>array(
'condition' => 'language=:language AND sid=:sid',
'params' => array(
':language' => $this->language,
':sid' => $this->sid
)
),
'message' => 'Question codes must be unique.'),

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Oct 21, 2013

Collaborator

I think we must set unique only of there are no parent.

Question and Sub-question have same models, no ?

This comment has been minimized.

Copy link
@SamMousa

SamMousa via email Oct 21, 2013

Author Contributor

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Oct 21, 2013

Collaborator

Great :)

Not sure it's linked: but have a problem with new suestion : no sub question added automtically.
Seems Question::model()->findAllByAttributes (line 467 of controllers/questions) send array [0]{}
(What the difference betwwen 2.00 and 2.05 here ????)

Not have a great look actually but bug reporting after some other part.

array('title', 'match', 'pattern' => '/[a-z,A-Z][[:alnum:]]+/', 'message' => 'Question codes must start with a letter and may only contain alphanumeric characters.'),

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Jul 4, 2018

Collaborator

This broke import of TSV survey file, possibly.

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Jul 4, 2018

Collaborator

Please fix. ;)

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Jul 4, 2018

Collaborator

@olleharstedt I really think we can not come back when we allow code with number (and same code in survey).

We fix it here in

// Try to fix question title for valid question code enforcement
for lss, but for TSV : since it's easily editable by user : think it's best to show the error and don't import potentially broken survey, no ?

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Jul 4, 2018

Collaborator

Hi Denis, this was mostly a joke. I and Dominik were working with this problem, commited here: 0099036

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Jul 4, 2018

Collaborator

The fix is strange, SQ : seems to be Sub Question. But the rules about question not starting with number must be set only for question, not for subquestion :

if (!$this->parent_qid) {

Else : since i see bug fix rehappen after some year, i never sure it's a joke …

This comment has been minimized.

Copy link
@olleharstedt

olleharstedt Jul 4, 2018

Collaborator

Please try to reproduce the issue, you'll see what's wrong.

This comment has been minimized.

Copy link
@Shnoulle

Shnoulle Jul 5, 2018

Collaborator

See my comment : https://bugs.limesurvey.org/view.php?id=13845#c48453

can you provide the TSV file ?

array('other', 'in','range'=>array('Y','N'), 'allowEmpty'=>true),
array('mandatory', 'in','range'=>array('Y','N'), 'allowEmpty'=>true),
array('question_order','numerical', 'integerOnly'=>true,'allowEmpty'=>true),
Expand Down

0 comments on commit ae2a46c

Please sign in to comment.