Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation answer without EM, before put it in session #533

Closed

Conversation

Shnoulle
Copy link
Collaborator

@Shnoulle Shnoulle commented Sep 8, 2016

  • Staring point, because ALL question can be validated this way : see Shnoulle@b98d5ee#diff-796aba4d06254b9caea9d461e4ea80c9R8756
  • we can not use array filtering for this case : we filter by real answer in DB : but this can be done with _validateQuestion (if someone found a way to find the answer filtered in EM for single choice :/ )
  • Here use a Class var, another idea was to use a App->user->(s|g)estState system (in session) : i'm not sure about the good choice ..... Class var seems cleaner BUT : All LEM is put in session, then need to be resetted. Shnoulle@b98d5ee#diff-796aba4d06254b9caea9d461e4ea80c9R649
  • move the numeric test type at same place : we show the error to the user, really better

Something great with session : Exemple of situation

  1. Put something bad in a question
  2. Move previous : the answer is set to null, but you can proceed. No alert is shown (must do ? or not , think better is not , see point 3)
  3. Move next : you see again the question, but here with the error message : you know the answer is not saved.

In fact : except for decimal(30,10) : i think user must hack HTML to see the error. I think the class alert alert-danger is OK.
capture du 2016-09-08 09-24-43

I send a surey for testing : https://bugs.limesurvey.org/view.php?id=11611 . Another survey can be tested : https://bugs.limesurvey.org/view.php?id=10827

…stig value

Dev : staring point, because ALL question can be validated this way.
Dev: we can not use aray filtering for this case : we filter by real answer in DB
Dev: here use a Class var, another idea was to use a App->user->(s|g)estState system (in session)
Dev: move the numeric test type at same place : we show the error to the user, really better
@Shnoulle
Copy link
Collaborator Author

Shnoulle commented Sep 8, 2016

Shit ... must review something with "no answer"

Dev: use $value to be saved and not one in session
Dev: never show error for hidden question or not relevant (move previous, set not relevant, move next)
Dev: don't unset value for DECIMAL : this can be a not hacked system
Dev: then redo the same test before save in DB
/**
* Control value against value from survey : see #11611
*/
if ($qrel)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't just insert more code, but break out functions instead and call them.

In this case, if ($qrel) { doSomethingWithSomething(); }.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At start , i completely add a new function to make validation but .... in fact ProcessCurrentResponse is done for this : validation responses.

I can create a new function again with an hundred var .... (here

foreach($sgqas as $sgqa)
{
  if($this->getValidityPost($qid,$sqga,$qhidden,....);
  {
    $stringToParse .= Yii::app()->getController()->renderPartial('/survey/system/questionhelp/tips', array('qid'=>$qid,'vclass'=>'dberror alert alert-danger','vtip'  =>$this->invalidAnswerCore[$sgqa]), true);
    $qvalid=false;
  }
}

because actual function need $stringToParse and $qvalid to be updated.

And thid need to be done in $sgqa.

You ask me to rewrite completely _ValidateQuestion .

Copy link
Contributor

Choose a reason for hiding this comment

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

No, just that new code should preferably be put in separate functions.


/**
* return the actual validity string , and reset the variable used ($_SESSION)
* @param $sgqa : the SGQ (answer name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be

@param string $sgqa ...

(Better for Scrutinizer)

@Shnoulle
Copy link
Collaborator Author

@c-schmitz : big update : only one function. All this test must be move to Question Object when ready. Leave it for now. We can add some validation after (exemple array number / checkbox : only 1/0 and set 0 to '')

@Shnoulle
Copy link
Collaborator Author

Can we merge it to develop version ? (i redo the pull request , no issue, but only if we merge it).

I think of another solution too : only filter "possible code" : 5 alphanumeric caracter (by default), but allow it out of the list + add a "only in list" like "mandatory" system (in fact : i first started with this). And we can use arry_filter too more easily in this step.

@Shnoulle Shnoulle changed the base branch from master to answers_html October 27, 2016 00:25
@Shnoulle
Copy link
Collaborator Author

Move to 3.0.

BUT : need to be reviewed 2 different control

  • User entered value : OK must fix it completely and send alert
  • Hidden question (for example) not posted but set with Equation : must only filter silently .... and don't fix it completely ....

@Shnoulle Shnoulle closed this Oct 27, 2016
Shnoulle added a commit that referenced this pull request Nov 9, 2016
… in DB

Dev: redo #533
Dev: use another view for tip : biggest (it's really an error) + dismissable (no js system)
@Shnoulle Shnoulle deleted the master_fix11611_validating branch April 5, 2022 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants