diff --git a/docs/upgrades/upgrade_7.5.10-7.5.11.pl b/docs/upgrades/upgrade_7.5.10-7.5.11.pl index 2358f7b77..0d43ae115 100644 --- a/docs/upgrades/upgrade_7.5.10-7.5.11.pl +++ b/docs/upgrades/upgrade_7.5.10-7.5.11.pl @@ -814,9 +814,9 @@ EOSQL1 my $assetSth = $session->db->read('select distinct(assetId) from Product'); my $accessorySth = $session->db->read('select accessoryAssetId from Product_accessory where assetId=? order by sequenceNumber'); my $relatedSth = $session->db->read('select relatedAssetId from Product_related where assetId=? order by sequenceNumber'); - my $specificationSth = $session->db->read('select name, value, units from Product_specification where assetId=? order by sequenceNumber'); - my $featureSth = $session->db->read('select feature from Product_feature where assetId=? order by sequenceNumber'); - my $benefitSth = $session->db->read('select benefit from Product_benefit where assetId=? order by sequenceNumber'); + my $specificationSth = $session->db->read('select Product_specificationId as specificationId, name, value, units from Product_specification where assetId=? order by sequenceNumber'); + my $featureSth = $session->db->read('select Product_featureId as featureId, feature from Product_feature where assetId=? order by sequenceNumber'); + my $benefitSth = $session->db->read('select Product_benefitId as benefitId, benefit from Product_benefit where assetId=? order by sequenceNumber'); while (my ($assetId) = $assetSth->array) { ##For each assetId, get each type of collateral ##Convert the data to JSON and store it in Product with setCollateral (update) diff --git a/lib/WebGUI/Asset/Sku/Product.pm b/lib/WebGUI/Asset/Sku/Product.pm index f451dfaaa..adfaf3de7 100644 --- a/lib/WebGUI/Asset/Sku/Product.pm +++ b/lib/WebGUI/Asset/Sku/Product.pm @@ -196,7 +196,7 @@ sub definition { #------------------------------------------------------------------- -=head2 deleteCollateral ( tableName, index ) +=head2 deleteCollateral ( tableName, keyName, keyValue ) Deletes a row of collateral data. @@ -204,18 +204,25 @@ Deletes a row of collateral data. The name of the table you wish to delete the data from. -=head3 index +=head3 keyName -The index of the data to delete from the collateral. +The name of a key in the collateral hash. Typically a unique identifier for a given +"row" of collateral data. + +=head3 keyValue + +Along with keyName, determines which "row" of collateral data to delete. =cut sub deleteCollateral { my $self = shift; my $tableName = shift; - my $index = shift; + my $keyName = shift; + my $keyValue = shift; my $table = $self->getAllCollateral($tableName); - return unless (abs($index) <= $#{$table}); + my $index = $self->getCollateralDataIndex($table, $keyName, $keyValue); + return if $index == -1; splice @{ $table }, $index, 1; $self->setAllCollateral($tableName); } @@ -292,24 +299,67 @@ or null, then an empty hashRef will be returned to avoid strict errors. If the requested index is beyond the end of the collateral array, it also returns an empty hashRef. +=head3 keyName + +The name of a key in the collateral hash. Typically a unique identifier for a given +"row" of collateral data. + +=head3 keyValue + +Along with keyName, determines which "row" of collateral data to delete. +If this is equal to "new", then an empty hashRef will be returned to avoid +strict errors in the caller. If the requested data does not exist in the +collateral array, it also returns an empty hashRef. + =cut sub getCollateral { my $self = shift; my $tableName = shift; - my $index = shift; - if ($index eq "new" || $index eq "") { + my $keyName = shift; + my $keyValue = shift; + if ($keyValue eq "new" || $keyValue eq "") { return {}; } my $table = $self->getAllCollateral($tableName); - ##I don't know why you'd send this a negative index, - ##but it's valid perl. - if (abs($index) <= $#{$table}) { - return $table->[$index]; - } - else { - return {}; + my $index = $self->getCollateralDataIndex($table, $keyName, $keyValue); + return {} if $index == -1; + return $table->[$index]; +} + + +#------------------------------------------------------------------- + +=head2 getCollateralDataIndex ( table, keyName, keyValue ) + +Returns the index in a set of collateral where an element of the +data (keyName) has a certain value (keyValue). If the criteria +are not found, returns -1. + +=head3 table + +The collateral data to search + +=head3 keyName + +The name of a key in the collateral hash. + +=head3 keyValue + +The value that keyName should have to meet the criteria. + +=cut + +sub getCollateralDataIndex { + my $self = shift; + my $table = shift; + my $keyName = shift; + my $keyValue = shift; + for (my $index=0; $index <= $#{ $table }; $index++) { + return $index + if (exists $table->[$index]->{$keyName} and $table->[$index]->{$keyName} eq $keyValue ); } + return -1; } @@ -328,48 +378,6 @@ sub getConfiguredTitle { } -#------------------------------------------------------------------- - -=head2 getIndexedCollateralData ( tableName ) - -Same as getAllCollateral, except that an additional field, collateralIndex, -is added to each hash. This makes it easy to build loops for collateral -operations such as editing, deleting or moving. - -=head3 tableName - -The name of the table you wish to retrieve the data from. - -=cut - -sub getIndexedCollateralData { - my $self = shift; - my $tableName = shift; - my $table; - - if (exists $self->{_collateral}->{$tableName}) { - $table = $self->{_collateral}->{$tableName}; - } - else { - my $json = $self->get($tableName); - if ($json) { - $table = from_json($json); - } - else { - $table = []; - } - } - my $indexedTable = []; - for (my $index = 0; $index <= $#{ $table }; ++$index) { - my %row = %{ $table->[$index] }; - $row{collateralIndex} = $index; - push @{ $indexedTable }, \%row; - } - - return $indexedTable; -} - - #------------------------------------------------------------------- sub getFileIconUrl { my $self = shift; @@ -447,7 +455,7 @@ sub getWeight { #------------------------------------------------------------------- -=head2 moveCollateralDown ( tableName, index ) +=head2 moveCollateralDown ( tableName, keyName, keyValue ) Moves a collateral data item down one position. If called on the last element of the collateral array then it does nothing. @@ -456,18 +464,26 @@ collateral array then it does nothing. A string indicating the table that contains the collateral data. -=head3 index +=head3 keyName -The index of the collateral data to move down. +The name of a key in the collateral hash. Typically a unique identifier for a given +"row" of collateral data. + +=head3 keyValue + +Along with keyName, determines which "row" of collateral data to move. =cut sub moveCollateralDown { my $self = shift; my $tableName = shift; - my $index = shift; + my $keyName = shift; + my $keyValue = shift; my $table = $self->getAllCollateral($tableName); + my $index = $self->getCollateralDataIndex($table, $keyName, $keyValue); + return if $index == -1; return unless (abs($index) < $#{$table}); @{ $table }[$index,$index+1] = @{ $table }[$index+1,$index]; $self->setAllCollateral($tableName); @@ -476,7 +492,7 @@ sub moveCollateralDown { #------------------------------------------------------------------- -=head2 moveCollateralUp ( tableName, index ) +=head2 moveCollateralUp ( tableName, keyName, keyValue ) Moves a collateral data item up one position. If called on the first element of the collateral array then it does nothing. @@ -485,18 +501,26 @@ collateral array then it does nothing. A string indicating the table that contains the collateral data. -=head3 index +=head3 keyName -The index of the collateral data to move up. +The name of a key in the collateral hash. Typically a unique identifier for a given +"row" of collateral data. + +=head3 keyValue + +Along with keyName, determines which "row" of collateral data to move. =cut sub moveCollateralUp { my $self = shift; my $tableName = shift; - my $index = shift; + my $keyName = shift; + my $keyValue = shift; my $table = $self->getAllCollateral($tableName); + my $index = $self->getCollateralDataIndex($table, $keyName, $keyValue); + return if $index == -1; return unless $index && (abs($index) <= $#{$table}); @{ $table }[$index-1,$index] = @{ $table }[$index,$index-1]; $self->setAllCollateral($tableName); @@ -625,44 +649,55 @@ sub setAllCollateral { #----------------------------------------------------------------- -=head2 setCollateral ( tableName, keyName, properties ) +=head2 setCollateral ( tableName, keyName, keyValue, properties ) Performs and insert/update of collateral data for any wobject's collateral data. +Returns the id of the data that was set, even if a new row was added to the +data. =head3 tableName The name of the table to insert the data. -=head3 index +=head3 keyName -The index of the collateral data to set. If the index = "new", then a new entry -will be appended to the end of the collateral array. Otherwise, then the -appropriate entry will be overwritten the data. +The name of a key in the collateral hash. Typically a unique identifier for a given +"row" of collateral data. + +=head3 keyValue + +Along with keyName, determines which "row" of collateral data to set. +The index of the collateral data to set. If the keyValue = "new", then a +new entry will be appended to the end of the collateral array. Otherwise, +the appropriate entry will be overwritten with the new data. =head3 properties A hash reference containing the name/value pairs to be inserted into the collateral, using -the index mentioned above. +the criteria mentioned above. =cut sub setCollateral { my $self = shift; my $tableName = shift; - my $index = shift; + my $keyName = shift; + my $keyValue = shift; my $properties = shift; ##Note, since this returns a reference, it is actually updating ##the object cache directly. my $table = $self->getAllCollateral($tableName); - if ($index eq 'new' || $index eq '') { + if ($keyValue eq 'new' || $keyValue eq '') { + $properties->{$keyName} = $self->session->id->generate; push @{ $table }, $properties; $self->setAllCollateral($tableName); - return; + return $properties->{$keyName}; } - return unless (abs($index) <= $#{$table}); + my $index = $self->getCollateralDataIndex($table, $keyName, $keyValue); + return if $index == -1; $table->[$index] = $properties; $self->setAllCollateral($tableName); - return; + return $keyValue; } #------------------------------------------------------------------- @@ -822,7 +857,7 @@ sub www_deleteAccessoryConfirm { sub www_deleteBenefitConfirm { my $self = shift; return $self->session->privilege->insufficient() unless ($self->canEdit); - $self->deleteCollateral("benefitJSON", $self->session->form->process("bid")); + $self->deleteCollateral("benefitJSON", 'benefitId', $self->session->form->process("bid")); return ""; } @@ -875,41 +910,46 @@ sub www_deleteSpecificationConfirm { #------------------------------------------------------------------- sub www_editBenefit { my $self = shift; - my $bid = shift || $self->session->form->process("bid"); + my $bid = shift || $self->session->form->process('bid'); return $self->session->privilege->insufficient() unless ($self->canEdit); - my $data = $self->getCollateral("benefitJSON", ,$bid); + my $data = $self->getCollateral('benefitJSON', 'benefitId', $bid); my $i18n = WebGUI::International->new($self->session,'Asset_Product'); my $f = WebGUI::HTMLForm->new($self->session,-action=>$self->getUrl); $f->hidden( - -name => "bid", + -name => 'bid', -value => $bid, ); $f->hidden( - -name => "func", - -value => "editBenefitSave", + -name => 'func', + -value => 'editBenefitSave', ); $f->text( - -name => "benefit", + -name => 'benefit', -label => $i18n->get(51), -hoverHelp => $i18n->get('51 description'), -value => $data->{benefits}, ); $f->yesNo( - -name => "proceed", + -name => 'proceed', -label => $i18n->get(52), -hoverHelp => $i18n->get('52 description'), ); $f->submit; - return $self->getAdminConsole->render($f->print, "product benefit add/edit"); + return $self->getAdminConsole->render($f->print, 'product benefit add/edit'); } #------------------------------------------------------------------- sub www_editBenefitSave { my $self = shift; return $self->session->privilege->insufficient() unless ($self->canEdit); - $self->setCollateral('benefitJSON', $self->session->form->process('bid'), { - benefit => $self->session->form->process('benefit','text') - }); + $self->setCollateral( + 'benefitJSON', + 'benefitId', + $self->session->form->process('bid'), + { + benefit => $self->session->form->process('benefit','text') + }, + ); return '' unless($self->session->form->process('proceed')); return $self->www_editBenefit('new'); } @@ -1108,7 +1148,7 @@ sub www_moveAccessoryUp { sub www_moveBenefitDown { my $self = shift; return $self->session->privilege->insufficient() unless ($self->canEdit); - $self->moveCollateralDown("benefitJSON", $self->session->form->process("bid")); + $self->moveCollateralDown("benefitJSON", 'benefitId', $self->session->form->process("bid")); return ""; } @@ -1116,7 +1156,7 @@ sub www_moveBenefitDown { sub www_moveBenefitUp { my $self = shift; return $self->session->privilege->insufficient() unless ($self->canEdit); - $self->moveCollateralUp("benefitJSON", $self->session->form->process("bid")); + $self->moveCollateralUp("benefitJSON", 'benefitId', $self->session->form->process("bid")); return ""; } diff --git a/t/Asset/Sku/ProductCollateral.t b/t/Asset/Sku/ProductCollateral.t index 58dc6bec7..2c53ce420 100644 --- a/t/Asset/Sku/ProductCollateral.t +++ b/t/Asset/Sku/ProductCollateral.t @@ -47,203 +47,190 @@ my $product = $root->addChild({ isa_ok($product, "WebGUI::Asset::Sku::Product"); ok(! exists $product->{_collateral}, 'object cache does not exist yet'); -$product->setCollateral('variantsJSON', 'new', {a => 'aye', b => 'bee'}); +my $vid = $product->setCollateral('variantsJSON', 'vid', 'new', {a => 'aye', b => 'bee'}); isa_ok($product->{_collateral}, 'HASH', 'object cache created for collateral'); +ok($session->id->valid($vid), 'a valid id was generated for the new collateral entry'); my $json; $json = $product->get('variantsJSON'); my $jsonData = from_json($json); cmp_deeply( $jsonData, - [ {a => 'aye', b => 'bee' } ], + [ {a => 'aye', b => 'bee', vid => $vid } ], 'Correct JSON data stored when collateral is empty', ); my $dbJson = $session->db->quickScalar('select variantsJSON from Product where assetId=?', [$product->getId]); is($json, $dbJson, 'db updated with correct JSON'); -$product->setCollateral('variantsJSON', 'new', {c => 'see', d => 'dee'}); +my $vid2 = $product->setCollateral('variantsJSON', 'vid', 'new', {c => 'see', d => 'dee'}); my $collateral = $product->getAllCollateral('variantsJSON'); isa_ok($collateral, 'ARRAY', 'getAllCollateral returns an array ref'); cmp_deeply( $collateral, [ - {a => 'aye', b => 'bee' }, - {c => 'see', d => 'dee' }, + {a => 'aye', b => 'bee', vid => $vid }, + {c => 'see', d => 'dee', vid => $vid2 }, ], 'setCollateral: new always appends to the end', ); -$product->setCollateral('variantsJSON', 2, {a => 'see', b => 'dee'}); +$product->setCollateral('variantsJSON', 'vid', 'pollyWollyDoodle', {a => 'see', b => 'dee'}); cmp_deeply( $product->getAllCollateral('variantsJSON'), [ - {a => 'aye', b => 'bee' }, - {c => 'see', d => 'dee' }, + {a => 'aye', b => 'bee', vid => $vid }, + {c => 'see', d => 'dee', vid => $vid2 }, ], - 'setCollateral: out of range index does not work', + 'setCollateral: non-existant value of key does not set data', ); -$product->setCollateral('variantsJSON', 1, {a => 'see', b => 'dee'}); +$product->setCollateral('variantsJSON', 'brooks', $vid, {a => 'see', b => 'dee'}); cmp_deeply( $product->getAllCollateral('variantsJSON'), [ - {a => 'aye', b => 'bee' }, - {a => 'see', b => 'dee' }, + {a => 'aye', b => 'bee', vid => $vid }, + {c => 'see', d => 'dee', vid => $vid2 }, + ], + 'setCollateral: non-existant key with real value does not set data', +); + +$product->setCollateral('variantsJSON', 'vid', $vid2, {a => 'see', b => 'dee', vid => $vid2}); +cmp_deeply( + $product->getAllCollateral('variantsJSON'), + [ + {a => 'aye', b => 'bee', vid => $vid }, + {a => 'see', b => 'dee', vid => $vid2 }, ], 'setCollateral: set by index works', ); cmp_deeply( - $product->getCollateral('variantsJSON', "new"), + $product->getCollateral('variantsJSON', 'vid', "new"), {}, - 'getCollateral: index=new returns an empty hashref', + 'getCollateral: value=new returns an empty hashref', +); + +cmp_deeply( + $product->getCollateral('variantsJSON', 'vid'), + {}, + 'getCollateral: undef value returns an empty hashref', ); cmp_deeply( $product->getCollateral('variantsJSON'), {}, - 'getCollateral: undef index returns an empty hashref', + 'getCollateral: undef keyName returns an empty hashref', ); cmp_deeply( - $product->getCollateral('variantsJSON', 3), + $product->getCollateral('variantsJSON', 'vid', 'neverAValidGUID'), {}, - 'getCollateral: out of range index returns an empty hashref', + 'getCollateral: non-existant value with valid key returns an empty hashRef', ); cmp_deeply( - $product->getCollateral('variantsJSON', 1), - {a => 'see', b => 'dee' }, - 'getCollateral: get by index works', + $product->getCollateral('variantsJSON', 'xvid', $vid), + {}, + 'getCollateral: non-existant key with valid value returns an empty hashRef', ); cmp_deeply( - $product->getCollateral('variantsJSON', -1), - {a => 'see', b => 'dee' }, - 'getCollateral: negative index works', + $product->getCollateral('variantsJSON', 'vid', $vid2), + {a => 'see', b => 'dee', 'vid' => $vid2 }, + 'getCollateral: get by keyName and value works', ); -$product->setCollateral('variantsJSON', 'new', { a => 'alpha', b => 'beta'}); +my $vid3 = $product->setCollateral('variantsJSON', 'vid', 'new', { a => 'alpha', b => 'beta'}); -$product->deleteCollateral('variantsJSON', 1); +$product->deleteCollateral('variantsJSON', 'vid', $vid2); cmp_deeply( $product->getAllCollateral('variantsJSON'), [ - {a => 'aye', b => 'bee' }, - {a => 'alpha', b => 'beta' }, + {a => 'aye', b => 'bee', vid => $vid }, + {a => 'alpha', b => 'beta', vid => $vid3 }, ], - 'deleteCollateral: delete by index works', + 'deleteCollateral: delete by keyName and value works', ); -$product->deleteCollateral('variantsJSON', 4); +$product->deleteCollateral('variantsJSON', 'vid', 'andyDufresne'); cmp_deeply( $product->getAllCollateral('variantsJSON'), [ - {a => 'aye', b => 'bee' }, - {a => 'alpha', b => 'beta' }, + {a => 'aye', b => 'bee', vid => $vid }, + {a => 'alpha', b => 'beta', vid => $vid3 }, ], - 'deleteCollateral: out of range index does not delete', + 'deleteCollateral: non-existant value with valid key does not delete', ); -$product->deleteCollateral('variantsJSON', -1); +$product->deleteCollateral('variantsJSON', 'vid', $vid3); +my $vid4 = $product->setCollateral('variantsJSON', 'vid', 'new', { a => 'alligators', b => 'bursting'}); +my $vid5 = $product->setCollateral('variantsJSON', 'vid', 'new', { a => 'ah', b => 'bay'}); cmp_deeply( $product->getAllCollateral('variantsJSON'), [ - {a => 'aye', b => 'bee' }, - ], - 'deleteCollateral: negative index works', -); - -$product->setCollateral('variantsJSON', 'new', { a => 'alligators', b => 'bursting'}); -$product->setCollateral('variantsJSON', 'new', { a => 'ah', b => 'bay'}); -cmp_deeply( - $product->getAllCollateral('variantsJSON'), - [ - {a => 'aye', b => 'bee' }, - {a => 'alligators', b => 'bursting' }, - {a => 'ah', b => 'bay' }, + {a => 'aye', b => 'bee' , 'vid' => $vid }, + {a => 'alligators', b => 'bursting', 'vid' => $vid4 }, + {a => 'ah', b => 'bay', 'vid' => $vid5 }, ], 'setup correct for moving collateral', ); -$product->moveCollateralDown('variantsJSON', 1); +$product->moveCollateralDown('variantsJSON', 'vid', $vid4); cmp_deeply( $product->getAllCollateral('variantsJSON'), [ - {a => 'aye', b => 'bee' }, - {a => 'ah', b => 'bay' }, - {a => 'alligators', b => 'bursting' }, + {a => 'aye', b => 'bee' , 'vid' => $vid }, + {a => 'ah', b => 'bay', 'vid' => $vid5 }, + {a => 'alligators', b => 'bursting', 'vid' => $vid4 }, ], 'moveCollateralDown: worked', ); -$product->moveCollateralDown('variantsJSON', 3); +$product->moveCollateralDown('variantsJSON', 'vid', 'shawshankRedemption'); cmp_deeply( $product->getAllCollateral('variantsJSON'), [ - {a => 'aye', b => 'bee' }, - {a => 'ah', b => 'bay' }, - {a => 'alligators', b => 'bursting' }, + {a => 'aye', b => 'bee' , 'vid' => $vid }, + {a => 'ah', b => 'bay', 'vid' => $vid5 }, + {a => 'alligators', b => 'bursting', 'vid' => $vid4 }, ], - 'moveCollateralDown: can not move out of range collateral item', + 'moveCollateralDown: can not move non-existant collateral item', ); -$product->moveCollateralUp('variantsJSON', 1); +$product->moveCollateralUp('variantsJSON', 'vid', $vid5); cmp_deeply( $product->getAllCollateral('variantsJSON'), [ - {a => 'ah', b => 'bay' }, - {a => 'aye', b => 'bee' }, - {a => 'alligators', b => 'bursting' }, + {a => 'ah', b => 'bay', 'vid' => $vid5 }, + {a => 'aye', b => 'bee' , 'vid' => $vid }, + {a => 'alligators', b => 'bursting', 'vid' => $vid4 }, ], 'moveCollateralUp: worked', ); -$product->moveCollateralUp('variantsJSON', 0); +$product->moveCollateralUp('variantsJSON', 'vid', $vid5); cmp_deeply( $product->getAllCollateral('variantsJSON'), [ - {a => 'ah', b => 'bay' }, - {a => 'aye', b => 'bee' }, - {a => 'alligators', b => 'bursting' }, + {a => 'ah', b => 'bay', 'vid' => $vid5 }, + {a => 'aye', b => 'bee' , 'vid' => $vid }, + {a => 'alligators', b => 'bursting', 'vid' => $vid4 }, ], 'moveCollateralUp: can not move the first collateral item in the array', ); -$product->moveCollateralUp('variantsJSON', 5); +$product->moveCollateralUp('variantsJSON', 'vid', 'brooksHadley'); cmp_deeply( $product->getAllCollateral('variantsJSON'), [ - {a => 'ah', b => 'bay' }, - {a => 'aye', b => 'bee' }, - {a => 'alligators', b => 'bursting' }, + {a => 'ah', b => 'bay', 'vid' => $vid5 }, + {a => 'aye', b => 'bee' , 'vid' => $vid }, + {a => 'alligators', b => 'bursting', 'vid' => $vid4 }, ], - 'moveCollateralUp: out of range index does not do anything', -); - -$product->setCollateral('variantsJSON', 2, { a => 'aoo', b => 'boo' }); - -cmp_deeply( - $product->getIndexedCollateralData('variantsJSON'), - [ - {a => 'ah', b => 'bay', collateralIndex => 0}, - {a => 'aye', b => 'bee', collateralIndex => 1}, - {a => 'aoo', b => 'boo', collateralIndex => 2}, - ], - 'getIndexedCollateralData: returns data structure with indeces', -); - -cmp_deeply( - $product->getAllCollateral('variantsJSON'), - [ - {a => 'ah', b => 'bay'}, - {a => 'aye', b => 'bee'}, - {a => 'aoo', b => 'boo'}, - ], - 'getIndexedCollateralData: does not affect asset data', + 'moveCollateralUp: cannot move up non-existant collateral item', ); @@ -255,10 +242,10 @@ my $product2 = $root->addChild({ title => "Bible", }); -$product2->setCollateral('variantsJSON', 'new', { s => 'scooby', d => 'doo'}); +my $vid6 = $product2->setCollateral('variantsJSON', 'vid', 'new', { s => 'scooby', d => 'doo'}); cmp_deeply( - $product2->getCollateral('variantsJSON', 0), - { s => 'scooby', d => 'doo'}, + $product2->getCollateral('variantsJSON', 'vid', $vid6), + { s => 'scooby', d => 'doo', vid => $vid6, }, 'Doing a set before get works okay', );