Improved Survey validation messages

This commit is contained in:
Patrick Donelan 2009-04-02 05:04:23 +00:00
parent 9ca83226c5
commit be4133d72c
3 changed files with 62 additions and 64 deletions

View file

@ -684,7 +684,7 @@ sub processGotoExpression {
# Parse gotoExpressions one after the other (first one that's true wins)
foreach my $line (split /\n/, $expression) {
my $processed = $self->parseGotoExpression($line, $responses);
my $processed = WebGUI::Asset::Wobject::Survey::ResponseJSON->parseGotoExpression($self->session, $line, $responses);
next if !$processed;
@ -770,22 +770,22 @@ Replace each "$questionName" with its response value (from the $responses hashre
=cut
sub parseGotoExpression {
my $self = shift;
my ($expression, $responses) = validate_pos(@_, { type => SCALAR }, { type => HASHREF, default => {} });
my $class = shift;
my ($session, $expression, $responses) = validate_pos(@_, { isa => 'WebGUI::Session'}, { type => SCALAR }, { type => HASHREF, default => {} });
$self->session->log->debug("Parsing gotoExpression: $expression");
$session->log->debug("Parsing gotoExpression: $expression");
my ( $target, $rest ) = $expression =~ /\s* ([^:]+?) \s* : \s* (.*)/x;
$self->session->log->debug("Parsed as Target: [$target], Expression: [$rest]");
$session->log->debug("Parsed as Target: [$target], Expression: [$rest]");
if ( !defined $target ) {
$self->session->log->warn('Target undefined');
$session->log->warn('Target undefined');
return;
}
if ( !defined $rest || $rest eq q{} ) {
$self->session->log->warn('Expression undefined');
$session->log->warn('Expression undefined');
return;
}
@ -794,7 +794,7 @@ sub parseGotoExpression {
$rest =~ s/\$$questionName/$response/g;
}
$self->session->log->debug("Processed as: $rest");
$session->log->debug("Processed as: $rest");
return {
target => $target,

View file

@ -1209,63 +1209,72 @@ sub validateSurvey{
#set up valid goto targets
my $gotoTargets = $self->getGotoTargets();
my $goodTargets;
for my $g(@$gotoTargets){ $$goodTargets{$g} = 1; }
my $duplicateTargets;
for my $g (@{$gotoTargets}) {
$goodTargets->{$g}++;
$duplicateTargets->{$g}++ if $goodTargets->{$g} > 1;
}
#step through each section validating it.
my $sections = $self->sections();
for(my $s = 0; $s <= $#$sections; $s++){
my $sNum = $s + 1;
my $section = $self->section([$s]);
if(! $self->validateGoto($section,$goodTargets)){
push(@messages,"Section $s does not have a valid GOTO target.");
push @messages,"Section $sNum does not have a valid Jump target.";
}
if(! $self->validateGotoInfiniteLoop($section)){
push(@messages,"Section $sNum jumps to itself.");
}
if(! $self->validateGotoExpression($section,$goodTargets)){
push(@messages,"Section $s does not have a valid GOTO Expression target.");
push(@messages,"Section $sNum does not have a valid Jump Expression.");
}
if(! $self->validateInfLoop($section)){
push(@messages,"Section $s jumps to itself.");
}
if(! $self->validateExpressionSyntax($section)){
push(@messages,"Section $s does not appear to have valid GOTO Expression syntax.");
if (my $var = $section->{variable}) {
if (my $count = $duplicateTargets->{$var}) {
push @messages, "Section $sNum variable name $var is re-used in $count other place(s).";
}
}
#step through each question validating it.
my $questions = $self->questions([$s]);
for(my $q = 0; $q <= $#$questions; $q++){
my $qNum = $q + 1;
my $question = $self->question([$s,$q]);
if(! $self->validateGoto($question,$goodTargets)){
push(@messages,"Section $s Question $q does not have a valid GOTO target.");
push(@messages,"Section $sNum Question $qNum does not have a valid Jump target.");
}
if(! $self->validateGotoInfiniteLoop($question)){
push(@messages,"Section $sNum Question $qNum jumps to itself.");
}
if(! $self->validateGotoExpression($question,$goodTargets)){
push(@messages,"Section $s Question $q does not have a valid GOTO Expression target.");
}
if(! $self->validateExpressionSyntax($question)){
push(@messages,"Section $s Question $q does not appear to have valid GOTO Expression syntax.");
push(@messages,"Section $sNum Question $qNum does not have a valid Jump Expression.");
}
if($#{$question->{answers}} < 0){
push(@messages,"Section $s Question $q does not have any answers.");
push(@messages,"Section $sNum Question $qNum does not have any answers.");
}
if(! $question->{text} =~ /\w/){
push(@messages,"Section $s Question $q does not have any text.");
push(@messages,"Section $sNum Question $qNum does not have any text.");
}
if(! $self->validateInfLoop($question)){
push(@messages,"Section $s Question $q jumps to itself.");
if (my $var = $question->{variable}) {
if (my $count = $duplicateTargets->{$var}) {
push @messages, "Section $sNum Question $qNum variable name $var is re-used in $count other place(s).";
}
}
#step through each answer validating it.
my $answers = $self->answers([$s,$q]);
for(my $a = 0; $a <= $#$answers; $a++){
my $aNum = $a + 1;
my $answer = $self->answer([$s,$q,$a]);
if(! $self->validateGoto($answer,$goodTargets)){
push(@messages,"Section $s Question $q Answer $a does not have a valid GOTO target.");
push(@messages,"Section $sNum Question $qNum Answer $aNum does not have a valid Jump target.");
}
if(! $self->validateExpressionSyntax($answer)){
push(@messages,"Section $s Question $q Answer $a does not appear to have valid GOTO Expression syntax.");
if(! $self->validateGotoInfiniteLoop($answer)){
push(@messages,"Section $sNum Question $qNum Answer $aNum jumps to itself.");
}
if(! $self->validateGotoExpression($answer,$goodTargets)){
push(@messages,"Section $s Question $q Answer $a does not have a valid GOTO Expression target.");
}
if(! $self->validateInfLoop($answer)){
push(@messages,"Section $s Question $q Answer $a jumps to itself.");
push(@messages,"Section $sNum Question $qNum Answer $aNum does not have a valid Jump Expression.");
}
}
}
@ -1274,17 +1283,15 @@ sub validateSurvey{
return \@messages;
}
sub validateExpressionSyntax{
sub validateGoto{
my $self = shift;
my $object = shift;
if($object->{gotoExpression} =~ /\w/){
my ( $target, $rest ) = $object->{gotoExpression} =~ /\s* ([^:]+?) \s* : \s* (.*)/x;
return 0 if(! defined $target or ! defined $rest);
}
my $goodTargets = shift;
return 0 if($object->{goto} =~ /\w/ && ! exists($goodTargets->{$object->{goto}}));
return 1;
}
sub validateInfLoop{
sub validateGotoInfiniteLoop{
my $self = shift;
my $object = shift;
return 0 if($object->{goto} =~ /\w/ and $object->{goto} eq $object->{variable});
@ -1294,20 +1301,10 @@ sub validateInfLoop{
sub validateGotoExpression{
my $self = shift;
my $object = shift;
my $goodTargets = shift;
if($object->{gotoExpression} =~ /\w/ and $object->{gotoExpression} =~ /\s*?(\w*)/){
my $tar = $1;
return 0 if(! exists $goodTargets->{$1});
}
return 1;
}
sub validateGoto{
my $self = shift;
my $object = shift;
my $goodTargets = shift;
return 0 if($object->{goto} =~ /\w/ && ! exists($goodTargets->{$object->{goto}}));
return 1;
return 1 unless $object->{gotoExpression} =~ /\w/;
# The best we can do is return true/false on whether the gotoExpression parses
return WebGUI::Asset::Wobject::Survey::ResponseJSON->parseGotoExpression($self->session, $object->{gotoExpression});
}
=head2 section ($address)

View file

@ -326,28 +326,29 @@ is($rJSON->lastResponse(), 5, 'goto: finds first if there are duplicates');
# parseGotoExpression
#
####################################################
throws_ok { $rJSON->parseGotoExpression() } 'WebGUI::Error::InvalidParam', 'processGotoExpression takes exception to empty arguments';
is($rJSON->parseGotoExpression(q{}),
my $c = 'WebGUI::Asset::Wobject::Survey::ResponseJSON';
throws_ok { $c->parseGotoExpression($session, ) } 'WebGUI::Error::InvalidParam', 'processGotoExpression takes exception to empty arguments';
is($c->parseGotoExpression($session, q{}),
undef, '.. and undef with empty expression');
is($rJSON->parseGotoExpression('blah-dee-blah-blah'),
is($c->parseGotoExpression($session, 'blah-dee-blah-blah'),
undef, '.. and undef with duff expression');
is($rJSON->parseGotoExpression(':'),
is($c->parseGotoExpression($session, ':'),
undef, '.. and undef with missing target');
is($rJSON->parseGotoExpression('t1:'),
is($c->parseGotoExpression($session, 't1:'),
undef, '.. and undef with missing expression');
cmp_deeply($rJSON->parseGotoExpression('t1: 1'),
cmp_deeply($c->parseGotoExpression($session, 't1: 1'),
{ target => 't1', expression => '1'}, 'works for simple numeric expression');
cmp_deeply($rJSON->parseGotoExpression('t1: 1 - 23 + 456 * (78 / 9.0)'),
cmp_deeply($c->parseGotoExpression($session, 't1: 1 - 23 + 456 * (78 / 9.0)'),
{ target => 't1', expression => '1 - 23 + 456 * (78 / 9.0)'}, 'works for expression using all algebraic tokens');
cmp_deeply($rJSON->parseGotoExpression('t1: 1 != 3 <= 4 >= 5'),
cmp_deeply($c->parseGotoExpression($session, 't1: 1 != 3 <= 4 >= 5'),
{ target => 't1', expression => '1 != 3 <= 4 >= 5'}, q{..works with other ops too});
cmp_deeply($rJSON->parseGotoExpression('t1: $q1 + $q2 * $q3 - 4', { q1 => 11, q2 => 22, q3 => 33}),
cmp_deeply($c->parseGotoExpression($session, 't1: $q1 + $q2 * $q3 - 4', { q1 => 11, q2 => 22, q3 => 33}),
{ target => 't1', expression => '11 + 22 * 33 - 4'}, 'substitues q for value');
cmp_deeply($rJSON->parseGotoExpression('t1: $a silly var name * 10 + $another var name', { 'a silly var name' => 345, 'another var name' => 456}),
cmp_deeply($c->parseGotoExpression($session, 't1: $a silly var name * 10 + $another var name', { 'a silly var name' => 345, 'another var name' => 456}),
{ target => 't1', expression => '345 * 10 + 456'}, '..it even works for vars with spaces in their names');
cmp_deeply($rJSON->parseGotoExpression('t1: ($A < 4) and ($B < 4) or ($B > 6) && 1 || 1', { A => 2, B => 3}),
cmp_deeply($c->parseGotoExpression($session, 't1: ($A < 4) and ($B < 4) or ($B > 6) && 1 || 1', { A => 2, B => 3}),
{ target => 't1', expression => '(2 < 4) and (3 < 4) or (3 > 6) && 1 || 1'}, 'Boolean expressions ok');
cmp_deeply($rJSON->parseGotoExpression('t1: $a = 1; $a++; $a > 1'),
cmp_deeply($c->parseGotoExpression($session, 't1: $a = 1; $a++; $a > 1'),
{ target => 't1', expression => '$a = 1; $a++; $a > 1'}, 'Assignment and compound statements ok too');
####################################################