Skip to content

Commit

Permalink
Dev: create $qid and $other var for checkValidityAnswer (#1266)
Browse files Browse the repository at this point in the history
Dev: this don't change anything, but seems more understandable ?
Dev: alternaive : $qid = null;
Dev: if(!empty($qinfo['qid']) { $qid = $qinfo['qid'];}
Dev: if(empty($qid) && !empty($qinfo['info']['qid']) { …
  • Loading branch information
Shnoulle authored and dominikvitt committed Apr 23, 2019
1 parent 31b1111 commit aaf5ebf
Showing 1 changed file with 16 additions and 19 deletions.
35 changes: 16 additions & 19 deletions application/helpers/expressions/em_manager_helper.php
Expand Up @@ -10160,17 +10160,14 @@ private static function checkValidityAnswer($type,$value,$sgq,$qinfo,$set = true
{
if(!empty($value))
{
// create array keys if they don't exist, because code below expect them
// it is used only in a case when default values are loaded on page
if (!array_key_exists('info', $qinfo)){
if (array_key_exists('qid', $qinfo)){
$qinfo['info']['qid'] = $qinfo['qid'];
}
if (array_key_exists('other', $qinfo)){
$qinfo['info']['other'] = $qinfo['other'];
}

/* Find qid in different situation */
$qid = !empty($qinfo['qid']) ? $qinfo['qid'] : (!empty($qinfo['info']['qid']) ? $qinfo['info']['qid'] : null);
if(empty($qid)) {
// No qid ? No way to check ?
return;
}
$other = !empty($qinfo['other']) ? $qinfo['other'] : (!empty($qinfo['info']['other']) ? $qinfo['info']['other'] : null);

/* This function is called by a static function , then set it to static .... */
$LEM =& LimeExpressionManager::singleton();
// Using language to find some valid value : set it to an existing language of this survey (can be Survey::model()->findByPk($LEM->sessid)->language too)
Expand All @@ -10191,15 +10188,15 @@ private static function checkValidityAnswer($type,$value,$sgq,$qinfo,$set = true
{
if($value=="-oth-")
{
if($qinfo['info']['other']!='Y')
if($other!='Y')
{
$LEM->addValidityString($sgq,$value,gT("%s is an invalid value for this question"),$set);
return false;
}
}
else
{
if(!Answer::model()->getAnswerFromCode($qinfo['info']['qid'],$value,$language))
if(!Answer::model()->getAnswerFromCode($qid,$value,$language))
{
$LEM->addValidityString($sgq,$value,gT("%s is an invalid value for this question"),$set);
return false;
Expand All @@ -10210,15 +10207,15 @@ private static function checkValidityAnswer($type,$value,$sgq,$qinfo,$set = true
case 'O': // List with comment
if(substr($sgq,-7)!='comment')
{
if(!Answer::model()->getAnswerFromCode($qinfo['info']['qid'],$value,$language))
if(!Answer::model()->getAnswerFromCode($qid,$value,$language))
{
$LEM->addValidityString($sgq,$value,gT("%s is an invalid value for this question"),$set);
return false;
}
}
break;
case 'F': // Array
if(!Answer::model()->getAnswerFromCode($qinfo['info']['qid'],$value,$language))
if(!Answer::model()->getAnswerFromCode($qid,$value,$language))
{
$LEM->addValidityString($sgq,$value,gT("%s is an invalid value for this question"),$set);
return false;
Expand Down Expand Up @@ -10259,15 +10256,15 @@ private static function checkValidityAnswer($type,$value,$sgq,$qinfo,$set = true
}
break;
case 'H': // Array by column
if(!Answer::model()->getAnswerFromCode($qinfo['info']['qid'],$value,$language))
if(!Answer::model()->getAnswerFromCode($qid,$value,$language))
{
$LEM->addValidityString($sgq,$value,gT("%s is an invalid value for this question"),$set);
return false;
}
break;
case '1': // Array dual scale
$scale=intval(substr($sgq,-1)); // Get the scale {SGQ}#0 or {SGQ}#1 actually
if(!Answer::model()->getAnswerFromCode($qinfo['info']['qid'],$value,$language,$scale))
if(!Answer::model()->getAnswerFromCode($qid,$value,$language,$scale))
{
$LEM->addValidityString($sgq,$value,gT("%s is an invalid value for this question"),$set);
return false;
Expand Down Expand Up @@ -10305,7 +10302,7 @@ private static function checkValidityAnswer($type,$value,$sgq,$qinfo,$set = true
}
break;
case 'R': // Ranking
if(!Answer::model()->getAnswerFromCode($qinfo['info']['qid'],$value,$language))
if(!Answer::model()->getAnswerFromCode($qid,$value,$language))
{
$LEM->addValidityString($sgq,$value,gT("%s is an invalid value for this question"),$set);
return false;
Expand All @@ -10328,14 +10325,14 @@ private static function checkValidityAnswer($type,$value,$sgq,$qinfo,$set = true
/* No validty control ? size ? */
break;
case 'M':
if($value!="Y" && (substr($sgq,-5)!='other' && $qinfo['info']['other']=='Y'))
if($value!="Y" && (substr($sgq,-5)!='other' && $other=='Y'))
{
$LEM->addValidityString($sgq,$value,gT("%s is an invalid value for this question"),$set);
return false;
}
break;
case 'P':
if(substr($sgq,-7)!='comment' && $value!="Y" && (substr($sgq,-5)!='other' && $qinfo['info']['other']=='Y'))
if(substr($sgq,-7)!='comment' && $value!="Y" && (substr($sgq,-5)!='other' && $other=='Y'))
{
$LEM->addValidityString($sgq,$value,gT("%s is an invalid value for this question"),$set);
return false;
Expand Down

12 comments on commit aaf5ebf

@olleharstedt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey, could you please merge this into develop? Really hard to figure out the merge conflicts! Thank you.

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Arg …

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@olleharstedt : the worst is controllers/admin/tokens;php …

@olleharstedt
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Shnoulle Yes, I didn't check blame for tokens, but it looked messy. Feel free to contact the commiter for help.

@Shnoulle
Copy link
Collaborator Author

@Shnoulle Shnoulle commented on aaf5ebf Apr 27, 2019

Choose a reason for hiding this comment

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

I didn't understand why in develop it's Answer::model()->getAnswerFromCode($qid,$value,$language) == null , it's same than !Answer::model()->getAnswerFromCode($qid,$value,$language)

Maybe for scrutinizer ?

@olleharstedt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good question. Don't know. :)

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I know …

  • master : getAnswerFromCode return an arry (can be empty)
  • develop : return string|null , and scrutinier have right : we must check with === or is_null since answer text can be empty.

@olleharstedt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch, Denis!

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's OK in master : [] == false == null == "" and ['0'=>''] != false

@olleharstedt
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You work on saturday ?

@olleharstedt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh no, just browsing around. ^^

Please sign in to comment.