Fixed #10520 - Survey responses get confused if survey structure modified

Survey responses are now revision-aware.
The survey structure gets revisioned automatically as necessary when changes
are made on the Edit Survey screen (as necessary ~~ a response exists
for the current revision)
This commit is contained in:
Patrick Donelan 2009-06-16 06:25:38 +00:00
parent 3e5d5804f6
commit d820d43760
6 changed files with 260 additions and 97 deletions

View file

@ -32,6 +32,7 @@ my $session = start(); # this line required
# upgrade functions go here
setDefaultIcalInterval($session);
makeSurveyResponsesVersionAware($session);
finish($session); # this line required
@ -55,6 +56,26 @@ sub setDefaultIcalInterval {
print "DONE!\n" unless $quiet;
}
sub makeSurveyResponsesVersionAware {
my $session = shift;
print "\tAdding revisionDate column to Survey_response table... " unless $quiet;
$session->db->write("alter table Survey_response add column revisionDate bigint(20) not null default 0");
print "\tDefaulting revisionDate on existing responses to current latest revision... " unless $quiet;
for my $assetId ($session->db->buildArray('select assetId from Survey_response')) {
$session->db->write(<<END_SQL, [ $assetId, $assetId]);
update Survey_response
set revisionDate = (
select max(revisionDate)
from Survey
where Survey.assetId = ?
)
where Survey_response.assetId = ?
END_SQL
}
print "DONE!\n" unless $quiet;
}
# -------------- DO NOT EDIT BELOW THIS LINE --------------------------------

View file

@ -410,7 +410,7 @@ sub responseJSON {
# See if we need to load responseJSON from the database
if (!defined $json) {
$json = $self->session->db->quickScalar( 'select responseJSON from Survey_response where assetId = ? and Survey_responseId = ?', [ $self->getId, $responseId ] );
$json = $self->session->db->quickScalar( 'select responseJSON from Survey_response where Survey_responseId = ?', [ $responseId ] );
}
# Instantiate the ResponseJSON instance, and store it
@ -753,6 +753,65 @@ sub www_graph {
#-------------------------------------------------------------------
=head2 submitObjectEdit ( $params )
Called by L<www_submitObjectEdit> when an edit is submitted to a survey object.
A new revision of this Survey object will be created automatically if any responses exist for the current
revision (completed or in-progress). This ensures that the revision bound to a response never changes once
a response has been started.
=head3 params
The updated params of the object. If the special hash keys "delete", "copy", "removetype" or "addtype" are present,
these special actions will be carried out by delegating to e.g. L<deleteObject>, L<copyObject>, etc..
=cut
sub submitObjectEdit {
my $self = shift;
my $params = shift || {};
# Id is made up of at most: sectionIndex-questionIndex-answerIndex
my @address = split /-/, $params->{id};
# We will create a new revision if any responses exist for the current revision
my $responses
= $self->session->db->quickScalar(
'select count(*) from Survey_response where assetId = ? and revisionDate = ?',
[ $self->getId, $self->get('revisionDate') ] );
# Get a reference to the Survey instance that we want to perform updates on
my $survey = $self;
if ($responses) {
$self->session->log->debug( "Creating a new revision, $responses responses exist for the current revision "
. $self->get('revisionDate') );
$survey = $self->addRevision;
}
# See if any special actions were requested..
if ( $params->{delete} ) {
return $survey->deleteObject( \@address );
}
elsif ( $params->{copy} ) {
return $survey->copyObject( \@address );
}
elsif ( $params->{removetype} ) {
return $survey->removeType( \@address );
}
elsif ( $params->{addtype} ) {
return $survey->addType( $params->{addtype}, \@address );
}
# Update the addressed object (and have it automatically persisted)
$survey->surveyJSON_update( \@address, $params );
# Return the updated Survey structure
return $survey->www_loadSurvey( { address => \@address } );
}
#-------------------------------------------------------------------
=head2 www_submitObjectEdit ( )
This is called when an edit is submitted to a survey object. The POST should contain the id and updated params
@ -765,32 +824,13 @@ See L<WebGUI::Asset::Wobject::Survey::ResponseJSON/sectionIndex>.
sub www_submitObjectEdit {
my $self = shift;
return $self->session->privilege->insufficient()
unless $self->session->user->isInGroup( $self->get('groupToEditSurvey') );
my $params = $self->session->form->paramsHashRef();
# Id is made up of at most: sectionIndex-questionIndex-answerIndex
my @address = split /-/, $params->{id};
# See if any special actions were requested..
if ( $params->{delete} ) {
return $self->deleteObject( \@address );
}
elsif ( $params->{copy} ) {
return $self->copyObject( \@address );
}elsif( $params->{removetype} ){
return $self->removeType(\@address);
}elsif( $params->{addtype} ){
return $self->addType($params->{addtype},\@address);
}
# Update the addressed object (and have it automatically persisted)
$self->surveyJSON_update( \@address, $params );
# Return the updated Survey structure
return $self->www_loadSurvey( { address => \@address } );
return $self->submitObjectEdit($params);
}
#-------------------------------------------------------------------
@ -1336,8 +1376,17 @@ sub getResponseDetails {
# This includes abnormal finishes such as timeouts and restarts
my $isCompleteClause = defined $isComplete ? "isComplete = $isComplete" : 'isComplete > 0';
$responseId
||= $self->session->db->quickScalar("select Survey_responseId from Survey_response where userId = ? and assetId = ? and $isCompleteClause order by endDate desc limit 1", [ $userId, $self->getId ]);
if (!$responseId) {
($responseId, my $revisionDate)
= $self->session->db->quickArray(
"select Survey_responseId, revisionDate from Survey_response where userId = ? and assetId = ? and $isCompleteClause order by endDate desc limit 1",
[ $userId, $self->getId ]);
if ($responseId && $revisionDate != $self->get('revisionDate')) {
$self->session->log->debug("Revision Date $revisionDate for retrieved responseId $responseId does not match instantiated object "
. $self->getId . " revision date " . $self->get('revisionDate') . ". getResponseDetails could possibly do weird things.");
}
}
if (!$responseId) {
$self->session->log->debug("ResponseId not found");
@ -1391,7 +1440,8 @@ sub getResponseDetails {
=head2 newByResponseId ( responseId )
Class method. Instantiates a Survey instance from the given L<"responseId">, and loads the
user response into the Survey instance.
user response into the Survey instance. The Survey object returned will be the revision
bound to the response.
=head3 responseId
@ -1403,8 +1453,8 @@ sub newByResponseId {
my $class = shift;
my ($session, $responseId) = validate_pos(@_, {isa => 'WebGUI::Session'}, { type => SCALAR });
my ($assetId, $userId) = $session->db->quickArray('select assetId, userId from Survey_response where Survey_responseId = ?',
[$responseId]);
my ($assetId, $revisionDate, $userId)
= $session->db->quickArray('select assetId, revisionDate, userId from Survey_response where Survey_responseId = ?', [$responseId]);
if (!$assetId) {
$session->log->warn("ResponseId not bound to valid assetId: $responseId");
@ -1416,7 +1466,7 @@ sub newByResponseId {
return;
}
if (my $survey = $class->new($session, $assetId)) {
if (my $survey = $class->new($session, $assetId, 'WebGUI::Asset::Wobject::Survey', $revisionDate)) {
# Set the responseId manually rather than calling $self->responseId so that we
# can load a response regardless of whether it's marked isComplete
$survey->{responseId} = $responseId;
@ -1445,7 +1495,15 @@ sub www_takeSurvey {
return;
}
my $out = $self->processTemplate( {}, $self->get('surveyTakeTemplateId') );
# The template needs to know what Survey revisionDate is bound to the response, so that
# it can ask for questions for the appropriate Survey revision
# We don't mind if the revisionDate for the retrieved response doesn't match the revisionDate
# for this Survey object, because this www_ method simply returns the shell that is used to
# retrieve the actual Survey data (using the appropriate revisionDate url param)
my $responseId = $self->responseId({ignoreRevisionDate => 1});
my $revision = $self->session->db->quickScalar("select revisionDate from Survey_response where Survey_responseId = ?", [ $responseId ]);
my $out = $self->processTemplate( { revision => $revision }, $self->get('surveyTakeTemplateId') );
return $self->processStyle($out);
}
@ -1655,25 +1713,8 @@ sub www_loadQuestions {
my @questions;
eval { @questions = $self->responseJSON->nextQuestions(); };
# # Logical sections cause nextResponse to move when nextQuestions is called, so
# # persist and changes, and repeat the surveyEnd check in case we are now at the end
# $self->persistResponseJSON();
# if ( $self->responseJSON->surveyEnd() ) {
# $self->session->log->debug('surveyEnd, probably as a result of a Logical Section');
# if ( $self->get('quizModeSummary') ) {
# if(! $self->session->form->param('shownsummary')){
# my ($summary,$html) = $self->getSummary();
# my $json = to_json( { type => 'summary', summary => $summary, html => $html });
# $self->session->http->setMimeType('application/json');
# return $json;
# }
# }
# return $self->surveyEnd();
# }
my $section = $self->responseJSON->nextResponseSection();
#return $self->prepareShowSurveyTemplate($section,$questions);
$section->{id} = $self->responseJSON->nextResponseSectionIndex();
$section->{wasRestarted} = $wasRestarted;
@ -1928,27 +1969,42 @@ A value of isComplete to filter against (defaults to isComplete = 0)
If a responseId does not already exist, do not create one (default is to create an new responseId)
=head4 ignoreRevisionDate
Ignore the fact that the revisionDate bound to the retrieved response does not match the revisionDate for this Survey instance.
=cut
sub responseId {
my $self = shift;
my %opts = validate( @_, { userId => 0, isComplete => 0, noCreate => 0 } );
my %opts = validate( @_, { userId => 0, isComplete => 0, noCreate => 0, ignoreRevisionDate => 0 } );
my $userId = $opts{userId} || $self->session->user->userId;
my $isComplete = $opts{isComplete};
my $noCreate = $opts{noCreate};
my $ignoreRevisionDate = $opts{ignoreRevisionDate};
my $user = WebGUI::User->new( $self->session, $userId );
my $ip = $self->session->env->getIp;
my $responseId = $self->{responseId};
return $responseId if $responseId;
# If a cached responseId doesn't exist, get the current in-progress response from the db
# By default, get current response (e.g. isComplete = 0)
my $isCompleteClause = defined $isComplete ? "isComplete = $isComplete" : 'isComplete = 0';
$responseId ||= $self->session->db->quickScalar(
"select Survey_responseId from Survey_response where userId = ? and assetId = ? and $isCompleteClause order by endDate desc limit 1",
[ $userId, $self->getId ]
);
if (!$responseId) {
($responseId, my $revisionDate) = $self->session->db->quickArray(
"select Survey_responseId, revisionDate from Survey_response where userId = ? and assetId = ? and $isCompleteClause order by endDate desc limit 1",
[ $userId, $self->getId ]
);
if (!$ignoreRevisionDate && $responseId && $revisionDate != $self->get('revisionDate')) {
$self->session->log->warn("Revision Date $revisionDate for retrieved responseId $responseId does not match instantiated object "
. $self->getId . " revision date " . $self->get('revisionDate') . ". Refusing to return response");
return;
}
}
if ( !$responseId && $noCreate ) {
$self->session->log->debug("ResponseId doesn't exist, but we were asked not to create a new one");
@ -1956,6 +2012,7 @@ sub responseId {
}
# If no current in-progress response exists, create one (as long as we're allowed to)
# N.B. Response is bound to current Survey revisionDate
if ( !$responseId ) {
my $maxResponsesPerUser = $self->get('maxResponsesPerUser');
my $takenCount = $self->takenCount( { userId => $userId } );
@ -1971,7 +2028,8 @@ sub responseId {
startDate => scalar time,
endDate => 0,
assetId => $self->getId,
anonId => undef
revisionDate => $self->get('revisionDate'),
anonId => undef,
}
);
@ -2833,9 +2891,11 @@ sub www_runTests {
return $self->session->privilege->insufficient()
unless $self->session->user->isInGroup( $self->get('groupToEditSurvey') );
# Manage response ourselves rather than doing it over and over per-test
$self->session->db->write( 'delete from Survey_response where assetId = ? and userId = ?',
[ $self->getId, $self->session->user->userId() ] );
# Remove any in-progress reponses for current user
$self->session->db->write( 'delete from Survey_response where assetId = ? and userId = ? and isComplete = 0',
[ $self->getId, $self->session->user->userId() ] );
# Manage responses ourselves rather than doing it over and over per-test
my $responseId = $self->responseId( { userId => $self->session->user->userId } )
or return $self->www_editTestSuite('Unable to start survey response');

View file

@ -18,7 +18,7 @@ my $session = WebGUI::Test->session;
#----------------------------------------------------------------------------
# Tests
my $tests = 29;
my $tests = 42;
plan tests => $tests + 1;
#----------------------------------------------------------------------------
@ -65,58 +65,61 @@ $survey->persistSurveyJSON;
$session->user( { user => $user } );
my $responseId = $survey->responseId;
my $s = WebGUI::Asset::Wobject::Survey->newByResponseId($session, $responseId);
is($s->getId, $survey->getId, 'newByResponseId returns same Survey');
is($s->get('maxResponsesPerUser'), 1, 'maxResponsesPerUser defaults to 1');
ok($s->canTakeSurvey, '..which means user can take survey');
{
my $s = WebGUI::Asset::Wobject::Survey->newByResponseId($session, $responseId);
is($s->getId, $survey->getId, 'newByResponseId returns same Survey');
}
is($survey->get('maxResponsesPerUser'), 1, 'maxResponsesPerUser defaults to 1');
ok($survey->canTakeSurvey, '..which means user can take survey');
is($survey->get('revisionDate'), $session->db->quickScalar('select revisionDate from Survey_response where Survey_responseId = ?', [$responseId]), 'Current revisionDate used');
# Complete Survey
$s->surveyEnd();
$survey->surveyEnd();
# Uncache canTake
delete $s->{canTake};
delete $s->{responseId};
ok(!$s->canTakeSurvey, 'Cannot take survey a second time (maxResponsesPerUser=1)');
cmp_deeply($s->responseId, undef, '..and similarly cannot get responseId');
delete $survey->{canTake};
delete $survey->{responseId};
ok(!$survey->canTakeSurvey, 'Cannot take survey a second time (maxResponsesPerUser=1)');
cmp_deeply($survey->responseId, undef, '..and similarly cannot get responseId');
# Change maxResponsesPerUser to 2
$s->update({maxResponsesPerUser => 2});
delete $s->{canTake};
ok($s->canTakeSurvey, '..but can take when maxResponsesPerUser increased to 2');
ok($s->responseId, '..and similarly can get responseId');
$survey->update({maxResponsesPerUser => 2});
delete $survey->{canTake};
ok($survey->canTakeSurvey, '..but can take when maxResponsesPerUser increased to 2');
ok($survey->responseId, '..and similarly can get responseId');
# Change maxResponsesPerUser to 0
$s->update({maxResponsesPerUser => 0});
delete $s->{canTake};
delete $s->{responseId};
ok($s->canTakeSurvey, '..and also when maxResponsesPerUser set to 0 (unlimited)');
ok($s->responseId, '..(and similarly for responseId)');
$survey->update({maxResponsesPerUser => 0});
delete $survey->{canTake};
delete $survey->{responseId};
ok($survey->canTakeSurvey, '..and also when maxResponsesPerUser set to 0 (unlimited)');
ok($survey->responseId, '..(and similarly for responseId)');
# Start a new response as another user
$s->update({maxResponsesPerUser => 1});
is($s->takenCount( { userId => 1 } ), 0, 'Visitor has no responses');
$survey->update({maxResponsesPerUser => 1});
is($survey->takenCount( { userId => 1 } ), 0, 'Visitor has no responses');
my $u = WebGUI::User->new( $session, 'new' );
WebGUI::Test->usersToDelete($u);
is($s->takenCount( { userId => $u->userId } ), 0, 'New user has no responses');
delete $s->{canTake};
delete $s->{responseId};
is($survey->takenCount( { userId => $u->userId } ), 0, 'New user has no responses');
delete $survey->{canTake};
delete $survey->{responseId};
$session->user( { userId => $u->userId } );
ok($s->canTakeSurvey, 'Separate counts for separate users');
ok($s->responseId, '..(and similarly for responseId)');
ok($survey->canTakeSurvey, 'Separate counts for separate users');
ok($survey->responseId, '..(and similarly for responseId)');
# Put things back to normal..
delete $s->{canTake};
delete $s->{responseId};
delete $survey->{canTake};
delete $survey->{responseId};
$session->user( { user => $user } );
# Restart the survey
$s->update({maxResponsesPerUser => 0});
$s->submitQuestions({
$survey->update({maxResponsesPerUser => 0});
$survey->submitQuestions({
'0-0-0' => 'this text ignored',
'0-1-0' => 'this text ignored',
});
cmp_deeply(
$s->responseJSON->responses,
$survey->responseJSON->responses,
superhashof(
{ '0-1-0' => {
'time' => num( time, 5 ),
@ -132,18 +135,18 @@ cmp_deeply(
);
# Test Restart
$s->surveyEnd( { restart => 1 } );
cmp_deeply($s->responseJSON->responses, {}, 'restart removes the in-progress response');
ok($responseId ne $s->responseId, '..and uses a new responseId');
$survey->surveyEnd( { restart => 1 } );
cmp_deeply($survey->responseJSON->responses, {}, 'restart removes the in-progress response');
ok($responseId ne $survey->responseId, '..and uses a new responseId');
# Test out exitUrl with an explicit
# Test out exitUrl with an explicit url
use JSON;
my $surveyEnd = $s->surveyEnd( { exitUrl => 'home' } );
my $surveyEnd = $survey->surveyEnd( { exitUrl => 'home' } );
cmp_deeply(from_json($surveyEnd), { type => 'forward', url => '/home' }, 'exitUrl works (it adds a slash for us)');
# Test out exitUrl using survey instance exitURL property
$s->update({ exitURL => 'getting_started'});
$surveyEnd = $s->surveyEnd( { exitUrl => undef } );
$survey->update({ exitURL => 'getting_started'});
$surveyEnd = $survey->surveyEnd( { exitUrl => undef } );
cmp_deeply(from_json($surveyEnd), { type => 'forward', url => '/getting_started' }, 'exitUrl works (it adds a slash for us)');
# www_jumpTo
@ -170,9 +173,65 @@ cmp_deeply(from_json($surveyEnd), { type => 'forward', url => '/getting_started'
}
}
# Response Revisioning
{
# Delete existing responses
$session->db->write('delete from Survey_response where assetId = ?', [$survey->getId]);
delete $survey->{responseId};
delete $survey->{surveyJSON};
my $surveyId = $survey->getId;
my $revisionDate = WebGUI::Asset->getCurrentRevisionDate($session, $surveyId);
ok($revisionDate, 'Revision Date initially defined');
# Modify Survey structure, new revision not created
$survey->submitObjectEdit({ id => "0", text => "new text"});
is($survey->surveyJSON->section([0])->{text}, 'new text', 'Survey updated');
is($session->db->quickScalar('select revisionDate from Survey where assetId = ?', [$surveyId]), $revisionDate, 'Revision unchanged');
# Push revisionDate into the past because we can't have 2 revision dates with the same epoch (this is very hacky)
$revisionDate--;
$session->stow->deleteAll();
WebGUI::Cache->new($session)->flush;
$session->db->write('update Survey set revisionDate = ? where assetId = ?', [$revisionDate, $surveyId]);
$session->db->write('update assetData set revisionDate = ? where assetId = ?', [$revisionDate, $surveyId]);
$session->db->write('update wobject set revisionDate = ? where assetId = ?', [$revisionDate, $surveyId]);
$survey = WebGUI::Asset->new($session, $surveyId);
isa_ok($survey, 'WebGUI::Asset::Wobject::Survey', 'Got back survey after monkeying with revisionDate');
is($session->db->quickScalar('select revisionDate from Survey where assetId = ?', [$surveyId]), $revisionDate, 'Revision date pushed back');
# Create new response
my $responseId = $survey->responseId;
is(
$session->db->quickScalar('select revisionDate from Survey_response where Survey_responseId = ?', [$responseId]),
$revisionDate,
'Pushed back revisionDate used for new response'
);
# Make another change, causing new revision to be automatically created
$survey->submitObjectEdit({ id => "0", text => "newer text"});
my $newerSurvey = WebGUI::Asset->new($session, $surveyId); # retrieve newer revision
isa_ok($newerSurvey, 'WebGUI::Asset::Wobject::Survey', 'After change, re-retrieved Survey instance');
is($newerSurvey->getId, $surveyId, '..which is the same survey');
is($newerSurvey->surveyJSON->section([0])->{text}, 'newer text', '..with updated text');
ok($newerSurvey->get('revisionDate') > $revisionDate, '..and newer revisionDate');
# Create another response (this one will use the new revision)
my $newUser = WebGUI::User->new( $session, 'new' );
WebGUI::Test->usersToDelete($newUser);
$session->user({ user => $newUser });
my $newResponseId = $survey->responseId;
is($newerSurvey->responseJSON->nextResponseSection()->{text}, 'newer text', 'New response uses the new text');
# And the punch line..
is($survey->responseJSON->nextResponseSection()->{text}, 'new text', '..wheras the original response uses the original text');
}
}
# Test visualization
eval 'use GraphViz';
SKIP: {
@ -190,6 +249,7 @@ like($storage->getFileContentsAsScalar($filename), qr{
}
#----------------------------------------------------------------------------
# Cleanup
END {

View file

@ -783,6 +783,5 @@ if (typeof Survey === "undefined") {
})();
YAHOO.util.Event.onDOMReady(function(){
// Survey.Comm.setUrl('/' + document.getElementById('assetPath').value);
Survey.Comm.callServer('', 'loadQuestions');
});

View file

@ -79,16 +79,39 @@ if (typeof Survey === "undefined") {
}
},
submitSummary: function(data,functionName){
var sUrl = "?func=loadQuestions&shownSummary=1";
var sUrl = "?func=loadQuestions;shownSummary=1";
var revision = Survey.Comm.getRevision();
if (revision) {
sUrl += ";revision=" + revision;
}
request(sUrl, this.callback, null, null, null);
},
getRevision: function() {
// Use the appropriate Survey response revision
var revision = parseInt(document.getElementById('surveyResponseRevision').value, 10);
if (!revision) {
YAHOO.log("Revision not found, bad template?");
}
return revision;
},
callServer: function(data, functionName, form, hasFile){
var postData;
if (!form) {
postData = "data=" + YAHOO.lang.JSON.stringify(data, data);
}
//var sUrl = this.url + "?func="+functionName;
var sUrl = "?func=" + functionName;
var revision = Survey.Comm.getRevision();
if (revision) {
sUrl += ";revision=" + revision;
}
request(sUrl, this.callback, postData, form, hasFile);
}
};