Skip to content

Commit

Permalink
Fixed issue #13845: Can't export and then import tab-separated text s…
Browse files Browse the repository at this point in the history 
…urvey file
  • Loading branch information
@dominikvitt
dominikvitt committed Jul 4, 2018
1 parent 7e9e5e0 commit 0099036
Showing 1 changed file with 10 additions and 1 deletion.
11 changes: 10 additions & 1 deletion application/helpers/expressions/em_manager_helper.php
View file
Expand Up @@ -10267,7 +10267,16 @@ static public function &TSVSurveyExport($sid)
$row = array();
$row['class'] = 'SQ';
$row['type/scale'] = 0;
$row['name'] = substr($varName,strlen($rootVarName)+1);

$subqName = substr($varName,strlen($rootVarName)+1);
// it breaks TSV survey import process if first character for name is numeric
// in such case, characters 'SQ' are added to the front of name, so validation can pass
if (preg_match('/^\d/', subqName) === 1){
$row['name'] = 'SQ'.subqName;
} else {
$row['name'] = subqName;
}

$row['relevance'] = $SQrelevance;
$row['text'] = $subqText;
$row['language'] = $lang;
Expand Down

14 comments on commit 0099036

@olleharstedt
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, what happened to the other fix for ranking question? Does import and activate survey work?

@Shnoulle
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment : the rules about question code not starting by a number are only for question, not sub question. Sub question starting by a number is allowed …

I‘m not at my office , but think a better fix is to set the parent_qid before save the question, no ?

@olleharstedt
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Subquestion codes are validated during tsv import, so it won't work. Feel free to test.

@dominikvitt
Copy link
Contributor Author

@dominikvitt dominikvitt commented on 0099036 Jul 5, 2018 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olleharstedt
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Don't forget to check the structure of the activated survey table (lime_survey_xxxxx) to make sure there are no duplicate subquestions or something.

@dominikvitt
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checked it.
There is no difference in survey table structure between surveys imported via lss or tsv file.
So it looks that it works okay.

@olleharstedt
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Good testing, Dominik! ⭐

@Shnoulle
Copy link
Collaborator

@Shnoulle Shnoulle commented on 0099036 Jul 5, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Subquestion codes are validated during tsv import, so it won't work. Feel free to test.

Yes , subquestion code validated, but rules didn't force to start with an alpha …

And without a tsv file : i can not understand. I see only one way : Sub question are created without parent question …

See the rules

For subquestion : there are only other and time that is disable, else : alphanum

LimeSurvey/application/models/Question.php

Line 167 in 4dcc3bd

$aRules[] = array('title', 'match', 'pattern' => '/^[[:alnum:]]*$/',

@Shnoulle
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one question : did you test your fix : https://bugs.limesurvey.org/view.php?id=13858 ?

@dominikvitt
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it before and it worked fine.
But now it looks like it's broken.
Don't worry, I'm building a new export function for tsv, so there won't be any differences between lss and tsv.

@Shnoulle
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$row['name'] = 'SQ'.subqName; this can NEVER work …

Else : rework totally TSV seems more a refactoring … since we don't have a TSV with ranking from 2.73 : we can not really test. But i'm sure its don't really broke survey … with the fix from the pull request …

@Shnoulle
Copy link
Collaborator

@Shnoulle Shnoulle commented on 0099036 Jul 18, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dominikvitt Please … time to revert this line and merge #1092

@olleharstedt
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted. All this code will be replaced by Dominik's work later.

@Shnoulle
Copy link
Collaborator

@Shnoulle Shnoulle commented on 0099036 Jul 18, 2018 •
edited by c-schmitz

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When refactoring : remember multi language files. We had big issues with it before.
And I think we must «silently» fix some parts on import: for example if a Question didn't have primary language : don't import it :) (or delete if after import (unsure if survey fixer fix this)

Please sign in to comment.