From f1dd08c13590f89e0b0c16a2b0f014fb94148345 Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Mon, 10 Jan 2011 16:00:21 -0800 Subject: [PATCH] If there are missing, required fields when saving data to a thing, do not write it to the db. Fixes bug #11996. --- docs/changelog/7.x.x.txt | 1 + lib/WebGUI/Asset/Wobject/Thingy.pm | 8 +- t/Asset/Wobject/Thingy/editThingDataSave.t | 110 +++++++++++++++++++++ 3 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 t/Asset/Wobject/Thingy/editThingDataSave.t diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index 1f0f0db0d..f440f5301 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -28,6 +28,7 @@ - fixed #11985: Search.pl should warn on bad assets - fixed #12008: Activity CleanLoginHistory is too slow - fixed #12004: SSO operation vulnerable to session fixation attacks + - fixed #11996: Mandatory field in Thingy saves empty 7.10.6 - fixed #11974: Toolbar icons unclickable in Webkit using HTML5 diff --git a/lib/WebGUI/Asset/Wobject/Thingy.pm b/lib/WebGUI/Asset/Wobject/Thingy.pm index 8e90bb9a6..5b468d59b 100644 --- a/lib/WebGUI/Asset/Wobject/Thingy.pm +++ b/lib/WebGUI/Asset/Wobject/Thingy.pm @@ -556,7 +556,9 @@ sub deleteThing { =head2 editThingDataSave ( ) -Saves a row of thing data and triggers the appropriate workflow triggers. +Saves a row of thing data and triggers the appropriate workflow triggers. Returns the id of the row created in +the database, and an array reference of errors from required fields and other sources. In there are errors, no data +is saved in the database, and the id returned in the empty string. =head3 thingId @@ -619,7 +621,6 @@ sub editThingDataSave { push (@errors,{ "error_message"=>$field->{label}." ".$i18n->get('is required error').".", }); - #$hadErrors = 1; } if ($field->{status} eq "hidden") { $fieldValue = $field->{defaultValue}; @@ -632,6 +633,9 @@ sub editThingDataSave { $thingData{$fieldName} = $fieldValue; } + if (@errors) { + return ('', \@errors); + } $newThingDataId = $self->setCollateral("Thingy_".$thingId,"thingDataId",\%thingData,0,0); # trigger workflow diff --git a/t/Asset/Wobject/Thingy/editThingDataSave.t b/t/Asset/Wobject/Thingy/editThingDataSave.t new file mode 100644 index 000000000..97e751c02 --- /dev/null +++ b/t/Asset/Wobject/Thingy/editThingDataSave.t @@ -0,0 +1,110 @@ +#------------------------------------------------------------------- +# WebGUI is Copyright 2001-2009 Plain Black Corporation. +#------------------------------------------------------------------- +# Please read the legal notices (docs/legal.txt) and the license +# (docs/license.txt) that came with this distribution before using +# this software. +#------------------------------------------------------------------- +# http://www.plainblack.com info@plainblack.com +#------------------------------------------------------------------- + +use FindBin; +use strict; +use lib "$FindBin::Bin/../../../lib"; + +##The goal of this test is to test editThingDataSave, particularly those things not tested in Thingy.t + +use WebGUI::Test; +use WebGUI::Session; +use Test::More tests => 6; # increment this value for each test you create +use Test::Deep; +use JSON; +use WebGUI::Asset::Wobject::Thingy; +use Data::Dumper; + +my $session = WebGUI::Test->session; + +# Do our work in the import node +my $node = WebGUI::Asset->getImportNode($session); + +my $versionTag = WebGUI::VersionTag->getWorking($session); +$versionTag->set({name=>"Thingy Test"}); +WebGUI::Test->addToCleanup($versionTag); +my $thingy = $node->addChild({className=>'WebGUI::Asset::Wobject::Thingy'}); + +# Test adding a new Thing +my $i18n = WebGUI::International->new($session, "Asset_Thingy"); +my $groupIdEdit = $thingy->get("groupIdEdit"); +my %thingProperties = ( + thingId => "new", + label => $i18n->get('assetName'), + editScreenTitle => $i18n->get('edit screen title label'), + editInstructions => '', + groupIdAdd => $groupIdEdit, + groupIdEdit => $groupIdEdit, + saveButtonLabel => $i18n->get('default save button label'), + afterSave => 'searchThisThing', + editTemplateId => "ThingyTmpl000000000003", + groupIdView => $groupIdEdit, + viewTemplateId => "ThingyTmpl000000000002", + defaultView => 'searchThing', + searchScreenTitle => $i18n->get('search screen title label'), + searchDescription => '', + groupIdSearch => $groupIdEdit, + groupIdExport => $groupIdEdit, + groupIdImport => $groupIdEdit, + searchTemplateId => "ThingyTmpl000000000004", + thingsPerPage => 25, +); + +my $thingId = $thingy->addThing(\%thingProperties,0); + +# Test adding a field + +my %fieldProperties = ( + thingId => $thingId, + fieldId => "new", + label => "Optional", + dateCreated => time(), + fieldType => "text", + status => "editable", + display => 1, +); + +my $field1Id = $thingy->addField(\%fieldProperties, 0); +$fieldProperties{status} = 'required'; +$fieldProperties{label} = 'Required'; +my $field2Id = $thingy->addField(\%fieldProperties, 0); + +# Test adding, editing, getting and deleting thing data + +my ($newThingDataId, $errors); +($newThingDataId, $errors) = $thingy->editThingDataSave($thingId, 'new', + { + "field_".$field1Id => 'test value', + "field_".$field2Id => 'test value', + }, +); + +cmp_deeply( + $errors, + [], + 'no errors for a valid thing data save' +); + +ok $session->id->valid($newThingDataId), 'valid GUID for saved field'; + +my $row_exists = $session->db->quickScalar('select count(*) from `Thingy_'.$thingId.'`'); +is $row_exists, 1, 'data written to the database'; + +($newThingDataId, $errors) = $thingy->editThingDataSave($thingId, 'new', + { + "field_".$field1Id => 'another test value', + }, +); + +is scalar @{ $errors }, 1, 'one error due to missing, required field'; +is $newThingDataId, '', '... no row id returned'; + +$row_exists = $session->db->quickScalar('select count(*) from `Thingy_'.$thingId.'`'); +is $row_exists, 1, '... no new data written to the datbase';