From 41ddd8c6c46cc054bd810e602b4c5259ae003093 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20B=C3=B6hmer?= Date: Sun, 20 Jul 2025 22:53:09 +0200 Subject: [PATCH] Refactor code for adjusting dish ingredients Closes #370. Change started as bugfix for that but I soon realized the old code was somewhat broken and entangled. - reduce code duplication - clearly split code for handling dish ingredient/item value - name methods more precisely - improve error handling - pass UnitConversionGraph object around even more to avoid rebuilding it --- .../Controller/Ajax/IngredientsEditor.pm | 4 +- lib/Coocook/Controller/Item.pm | 5 +- lib/Coocook/Schema/Result/DishIngredient.pm | 107 +++----- lib/Coocook/Schema/Result/Item.pm | 251 +++++++++--------- lib/Coocook/Schema/Result/PurchaseList.pm | 2 +- lib/Coocook/Schema/ResultSet/Dish.pm | 2 +- lib/Coocook/Schema/ResultSet/Item.pm | 17 +- t/controller_Item.t | 18 ++ t/schema_DishIngredient.t | 14 +- t/schema_Item.t | 51 ++-- 10 files changed, 246 insertions(+), 225 deletions(-) diff --git a/lib/Coocook/Controller/Ajax/IngredientsEditor.pm b/lib/Coocook/Controller/Ajax/IngredientsEditor.pm index e5173f0a9..4637c5dc7 100644 --- a/lib/Coocook/Controller/Ajax/IngredientsEditor.pm +++ b/lib/Coocook/Controller/Ajax/IngredientsEditor.pm @@ -148,8 +148,8 @@ sub delete_ingredient : POST Chained('base') PathPart('ingredients/delete') my $ingredient = $c->stash->{dish_or_recipe}->ingredients->find( $ajax_request->{id} ) or $c->detach('/error/object_not_found'); - if ( $ingredient->is_dish_ingredient and my $item = $ingredient->item ) { - $item->remove_ingredients( $c->project->unit_conversion_graph, $ingredient ); + if ( $ingredient->is_dish_ingredient ) { + $ingredient->delete_update_item(); } else { $ingredient->delete(); diff --git a/lib/Coocook/Controller/Item.pm b/lib/Coocook/Controller/Item.pm index 8dc821d75..edd38e6b0 100644 --- a/lib/Coocook/Controller/Item.pm +++ b/lib/Coocook/Controller/Item.pm @@ -47,10 +47,11 @@ sub convert : POST Chained('base') Args(0) RequiresCapability('edit_project') { # can't use numerical equivalence with == because of floating point math! if ( $new_total <= 0 or $abs_difference > 0.0001 ) { - $c->detach('/error/bad_request'); + $c->detach( '/error/bad_request', + ["New total doesn’t fit to conversion. Conversions might have been edited in the background."] ); } - my $new_item = $item->change_value_offset_unit( + my $new_item = $item->set_value_offset_unit( $item->value * $factor, #perltidy $item->offset * $factor, $unit->id diff --git a/lib/Coocook/Schema/Result/DishIngredient.pm b/lib/Coocook/Schema/Result/DishIngredient.pm index 7f901082e..e06837c49 100644 --- a/lib/Coocook/Schema/Result/DishIngredient.pm +++ b/lib/Coocook/Schema/Result/DishIngredient.pm @@ -63,18 +63,17 @@ __PACKAGE__->has_many( __PACKAGE__->meta->make_immutable; -sub assign_to_purchase_list ( $self, $list_id ) { +sub assign_to_purchase_list ( $self, $list_id, $ucg = undef ) { my $item; $self->txn_do( sub { $item = $self->result_source->schema->resultset('Item')->add_or_create( - { - purchase_list_id => $list_id, - article_id => $self->article_id, - unit_id => $self->unit_id, - value => $self->value, - } + purchase_list_id => $list_id, + article_id => $self->article_id, + unit_id => $self->unit_id, + value => $self->value, + ucg => $ucg, ); $self->update( { item_id => $item->id } ); @@ -86,97 +85,67 @@ sub assign_to_purchase_list ( $self, $list_id ) { =head2 delete_update_item() -=cut - -sub delete_update_item ($self) { - my $retval = $self->item_id && $self->set_value_update_item(0); - $self->delete(); - return $retval; -} - -=head2 set_value_update_item( $value, $ucg? ) +Returns C if still existing, otherwise empty list. =cut -sub set_value_update_item ( $self, $new_value, $ucg = undef ) { +sub delete_update_item ( $self, $ucg = undef ) { $self->txn_do( sub { - my $item = $self->item - or return $self->update( { value => $new_value } ); - - my $old_value = $self->value; - - if ( $old_value == 0 ) { - $self->update( { item_id => undef, value => $new_value } ); + $self->delete(); - my $purchase_list_id = $item->purchase_list_id; - - if ( $item->ingredients->count == 0 ) { # TODO use results_exist() - $item->delete(); - $item = undef; - } - - if ( $new_value > 0 ) { - return $self->assign_to_purchase_list($purchase_list_id); + if ( my $item = $self->item ) { + if ( $item->ingredients->results_exist ) { + return $item->delta_to_value_or_recalculate( $self->value => 0, $self->unit_id => undef, $ucg ); } - else { - return $item; - } - } - if ( $new_value == 0 and not $self->other_ingredients_on_item->results_exist ) { - $self->update( { item_id => undef, value => 0 } ); $item->delete(); - return; } - $self->update( { value => $new_value } ); + return; + } + ); +} - $ucg ||= $item->purchase_list->project->unit_conversion_graph; +=head2 set_value_update_item( $value, $ucg? ) - if ( my $factor = $ucg->factor_between_units( $self->unit_id => $item->unit_id ) ) { - if ( $self->other_ingredients_on_item->results_exist ) { - return $item->delta_to_value_offset( ( $new_value - $old_value ) * $factor ); - } - else { - return $item->set_value_offset( $new_value * $factor ); - } - } +Returns the updated C or an empty list if it doesn't exist. + +=cut - # no factor to item unit -> whole item needs to be rebuilt - return $item->update_from_ingredients(); +sub set_value_update_item ( $self, $new_value, $ucg = undef ) { + $self->txn_do( + sub { + my $old_value = $self->value; + $self->update( { value => $new_value } ); + $self->item or return; + $self->item->delta_to_value_or_recalculate( + $old_value => $new_value, + ( $self->unit_id ) x 2, $ucg + ); } ); } =head2 set_value_unit_update_item( $value, $unit, $ucg? ) +Returns the updated C or an empty list if it doesn't exist. + =cut sub set_value_unit_update_item ( $self, $new_value, $new_unit_id, $ucg = undef ) { $self->txn_do( sub { - my $item = $self->item - or return $self->update( { value => $new_value, unit_id => $new_unit_id } ); - - $ucg ||= $item->purchase_list->project->unit_conversion_graph; - my $old_value = $self->value; my $old_unit_id = $self->unit_id; - my $factor_ingredient = $ucg->factor_between_units( $old_unit_id => $new_unit_id ); - my $factor_item = $ucg->factor_between_units( $old_unit_id => $item->unit_id ); - - if ( $factor_ingredient and $factor_item ) { - $self->update( { value => $new_value, unit_id => $new_unit_id } ); - - my $delta = ( $new_value / $factor_ingredient - $old_value ) * $factor_item; - return $item->delta_to_value_offset($delta); - } - - $self->set_value_update_item(0); $self->update( { value => $new_value, unit_id => $new_unit_id } ); - $self->assign_to_purchase_list( $item->purchase_list_id ); + $self->item or return; + $self->item->delta_to_value_or_recalculate( + $old_value => $new_value, + $old_unit_id => $new_unit_id, + $ucg + ); } ); } diff --git a/lib/Coocook/Schema/Result/Item.pm b/lib/Coocook/Schema/Result/Item.pm index e80c8d2d2..87bd7d241 100644 --- a/lib/Coocook/Schema/Result/Item.pm +++ b/lib/Coocook/Schema/Result/Item.pm @@ -61,62 +61,70 @@ __PACKAGE__->meta->make_immutable; =encoding utf8 -=head2 delta_to_value_offset($delta) - -Adds a delta to the item’s value and adjusts the offset. -The delta needs to be given in the item’s unit. -Might delete the item if the new value will be zero. -Does B change any dish ingredients. -Returns the C object itself unless -it was deleted (then returns nothing). +=head2 delta_to_value_or_recalculate( $value1 => $value2, $unit1_id => $unit2_id, $ucg? ) + +Calculate the difference from C<$value1 $unit1> to C<$value2 $unit2> and +add/subtract the difference to/from the item’s value. +If the item has a positive offset (rounding difference) and the value is increased +or the item has a negative offset and the value is decreased, +the offset is reduced until the difference exceeds the offset. +If values cannot be converted to the item’s unit, +the item’s value is recalculated from its dish ingredients instead. +Does B change any dish ingredients on its own—any modifications to the dish ingredients must be done beforehand. +Returns the C if it still exists. +In case of recalculation the item might get deleted. =cut -sub delta_to_value_offset ( $self, $delta ) { - return if $delta == 0; - - my $value = $self->value + $delta; - - return - $value < 0 ? croak "Delta leads to negative value" - : $value == 0 ? $self->delete() - : $self->_set_value_offset( $delta => $value ); -} +sub delta_to_value_or_recalculate ( $self, $value1, $value2, $unit1_id, $unit2_id, $ucg = undef ) { + defined $value1 or croak "Value 1 must be defined"; + defined $value2 or croak "Value 2 must be defined"; + $unit1_id or $unit2_id or croak "Unit 1 and 2 must not be both undefined"; + ( $value1 == 0 or $unit1_id ) or croak "Unit for value 1 must be defined"; + ( $value2 == 0 or $unit2_id ) or croak "Unit for value 2 must be defined"; -=head2 set_value_offset($value) + if ( ( $value1 and $unit1_id != $self->unit_id ) + or ( $value2 and $unit2_id != $self->unit_id ) ) + { + $ucg ||= $self->purchase_list->project->unit_conversion_graph; -Sets the item’s value to C<$value> and adjusts the offset. -The new value needs to be given in the item’s unit. -Might delete the item if the new value will be zero. -Does B change any dish ingredients. + my $factor1 = + ( $value1 and $unit1_id != $self->unit_id ) + ? $ucg->factor_between_units( $unit1_id => $self->unit_id ) + : 1; -=cut + my $factor2 = + ( $value2 and $unit2_id != $self->unit_id ) + ? $ucg->factor_between_units( $unit2_id => $self->unit_id ) + : 1; -sub set_value_offset ( $self, $value ) { - $value >= 0 or croak "Negative value"; - my $delta = $value - $self->value; - return $self if $delta == 0; - return $self->_set_value_offset( $delta => $value ); -} + if ( !$factor1 or !$factor2 ) { + my $factor1_2 = + ( $unit1_id and $unit2_id ) ? $ucg->factor_between_units( $unit1_id => $unit2_id ) : undef; -sub _set_value_offset ( $self, $delta, $value ) { # TODO order of methods in this file - $self->set_column( value => $value ); - - my $offset = $self->offset; + if ( $factor1_2 and $value1 * $factor1_2 == $value2 ) { + return $self; # value1/2 cannot be convert to item unit but are equal + } - if ( ( $delta < 0 and $offset < 0 ) - or ( $delta > 0 and $offset > 0 ) ) # both are negative or both positive - { - if ( abs($delta) <= abs($offset) ) { # fill up offset if possible - $offset -= $delta; - return $self->update( { offset => $offset } ); + # difference cannot be converted to item's unit + return $self->calculate_value_from_ingredients(); } + + $value1 *= $factor1; + $value2 *= $factor2; } - $self->update( { offset => 0 } ); # otherwise clear offset + return $self->_delta_to_value_adjust_offset( $value2 - $value1 ); } -sub change_value_offset_unit ( $self, $value, $offset, $unit_id ) { +=head2 set_value_offset_unit( $value, $offset, $unit_id ) + +Set value, offset and unit_id of the item. Useful for conversion by 1 click. +Returns the original C or a different one of the original was merged into that. + +=cut + +sub set_value_offset_unit ( $self, $value, $offset, $unit_id ) { $self->txn_do( sub { my $existing_item = $self->result_source->resultset->find( @@ -166,7 +174,6 @@ sub remove_ingredients ( $self, $ucg, @ingredients_to_remove ) { $self->txn_do( sub { my $recalculate_value; - my $value = $self->value; for my $ingredient (@ingredients_to_remove) { $ingredient->item_id == $self->id @@ -176,71 +183,32 @@ sub remove_ingredients ( $self, $ucg, @ingredients_to_remove ) { next if $recalculate_value; - if ( defined( my $factor = $ucg->factor_between_units( $ingredient->unit_id => $self->unit_id ) ) ) - { - my $delta = $ingredient->value * $factor; - $delta >= 0 or croak "Negative dish ingredient value"; - - if ( my $offset = $self->offset ) { - if ( $offset < 0 ) { - if ( -1 * $offset <= $delta ) { - $offset += $delta; # reduce negative (!) offset by delta - } - else { - $offset = 0; - } - } - elsif ( $self->offset > 0 ) { # decreasing value makes positive offset invalid - $self->set_column( offset => 0 ); - } - else { die "code broken" } - } - - $value -= $delta; - $value < 0 and $recalculate_value = 1; - } - else { - $recalculate_value = 1; + if ( $ingredient->value == 0 ) { + next; # nothing to do } - } - if ($recalculate_value) { - $value = 0; - my $item_keeps_ingredients; + # same unit (factor == 1) or convertible unit + if ( my $factor = $ucg->factor_between_units( $ingredient->unit_id => $self->unit_id ) ) { + my $delta = -1 * $ingredient->value * $factor; - for my $ingredient ( $self->ingredients->all ) { # remaining ingredients - if ( defined( my $factor = $ucg->factor_between_units( $ingredient->unit_id => $self->unit_id ) ) ) - { - $value += $ingredient->value * $factor; - $item_keeps_ingredients = 1; - } - else { # ingredient is added to other/new item of same unit - my $item = $self->other_items->add_or_create( - { - purchase_list_id => $self->purchase_list_id, - article_id => $ingredient->article_id, - unit_id => $ingredient->unit_id, - value => $ingredient->value, - } - ); - $ingredient->update( { item_id => $item->id } ); + if ( $self->value + $delta > 0 ) { # new value < 0 is implausible + $self->_delta_to_value_adjust_offset($delta); + next; } } - if ($item_keeps_ingredients) { - $self->set_column( offset => 0 ); - } - else { - $self->delete(); - return; - } + $recalculate_value = 1; + } + + if ($recalculate_value) { + return $self->calculate_value_from_ingredients($ucg); } elsif ( not $self->ingredients->results_exist ) { $self->delete(); return; } - return $self->update( { value => $value } ); + return $self; } ); } @@ -253,27 +221,34 @@ Returns total, i.e. sum of the item's value and offset. sub total ($self) { return $self->value + $self->offset } -sub update_from_ingredients ( $self, $ucg = undef ) { +=head2 calculate_value_from_ingredients($ucg?) + +Updates this item’s value from its ingredients’ values. Always resets offset to zero. +Might assign ingredients to other existing or new purchase list items if they cannot +be converted to this item’s unit. +Returns the C itself if it still exists or an empty list +if no ingredients remained assigned to this item and the item has been deleted. + +=cut + +sub calculate_value_from_ingredients ( $self, $ucg = undef ) { $self->txn_do( sub { - my $item_value = 0; - my $remaining_items = 0; - my @dangling_ingredients = (); + my $item_value = 0; + my $remaining_ingredients = 0; + my @dangling_ingredients = (); $ucg ||= $self->purchase_list->project->unit_conversion_graph; for my $ingredient ( $self->ingredients->all ) { - - my $ingredient_value = $ingredient->value; - if ( $ingredient->value == 0 ) { - $remaining_items++; + $remaining_ingredients++; next; } if ( my $factor = $ucg->factor_between_units( $ingredient->unit_id => $self->unit_id ) ) { $item_value += $ingredient->value * $factor; - $remaining_items++; + $remaining_ingredients++; next; } @@ -281,32 +256,64 @@ sub update_from_ingredients ( $self, $ucg = undef ) { push @dangling_ingredients, $ingredient; } - if ( $remaining_items == 0 ) { + if (@dangling_ingredients) { + my @other_items = $self->other_items->all; + + # add dangling ingredients existing/new items with minimal number of new items + INGREDIENT: for my $ingredient (@dangling_ingredients) { + for my $other_item (@other_items) { + if ( my $factor = $ucg->factor_between_units( $ingredient->unit_id => $other_item->unit_id ) ) { + $ingredient->update( { item_id => $other_item->id } ); + $other_item->_delta_to_value_adjust_offset( $ingredient->value * $factor ); + next INGREDIENT; + } + } + + my $new_item = $ingredient->assign_to_purchase_list( $self->purchase_list_id, $ucg ); + push @other_items, $new_item; + } + } + + if ($remaining_ingredients) { + $self->update( { value => $item_value, offset => 0 } ); + return $self; + } + else { $self->delete(); return; } + } + ); +} + +=head2 _delta_to_value_adjust_offset($delta) + +Adds/subtracts <$delta> to/from the item’s value and adjusts the item’s offset. +The delta needs to be given in the item’s unit. +Does B change any dish ingredients. - my @items; +=cut - # add dangling ingredients as new items with minimal number of new items - INGREDIENT: for my $ingredient (@dangling_ingredients) { - for my $item (@items) { - if ( my $factor = $ucg->factor_between_units( $ingredient->unit_id => $item->unit_id ) ) { - $ingredient->update( { item_id => $item->id } ); - $item->update( { value => $item->value + $ingredient->value * $factor } ); - last INGREDIENT; +sub _delta_to_value_adjust_offset ( $self, $delta ) { + return $self if $delta == 0; - } - } + my $value = $self->value + $delta; + $value >= 0 or croak "Delta creates negative value"; - my $item = $ingredient->assign_to_purchase_list( $self->purchase_list_id ); - push @items, $item; - } + $self->set_column( value => $value ); - $self->update( { value => $item_value, offset => 0 } ); - return $self; + my $offset = $self->offset; + + if ( ( $delta < 0 and $offset < 0 ) + or ( $delta > 0 and $offset > 0 ) ) # both are negative or both positive + { + if ( abs($delta) <= abs($offset) ) { # fill up offset if possible + $offset -= $delta; + return $self->update( { offset => $offset } ); } - ); + } + + $self->update( { offset => 0 } ); # otherwise clear offset } 1; diff --git a/lib/Coocook/Schema/Result/PurchaseList.pm b/lib/Coocook/Schema/Result/PurchaseList.pm index 3052f4bcd..2e44629b9 100644 --- a/lib/Coocook/Schema/Result/PurchaseList.pm +++ b/lib/Coocook/Schema/Result/PurchaseList.pm @@ -146,7 +146,7 @@ sub move_items_ingredients ( $self, %args ) { or $items{$item_id}->remove_ingredients( $args{ucg}, @$ingredients ); for my $ingredient (@$ingredients) { - $ingredient->assign_to_purchase_list( $args{target_purchase_list}->id ); + $ingredient->assign_to_purchase_list( $args{target_purchase_list}->id, $args{ucg} ); } } diff --git a/lib/Coocook/Schema/ResultSet/Dish.pm b/lib/Coocook/Schema/ResultSet/Dish.pm index 22797692e..640551522 100644 --- a/lib/Coocook/Schema/ResultSet/Dish.pm +++ b/lib/Coocook/Schema/ResultSet/Dish.pm @@ -37,7 +37,7 @@ sub from_recipe ( $self, %args ) { ); if ( my $list_id = $recipe->project->default_purchase_list_id ) { - $dish_ingredient->assign_to_purchase_list($list_id); + $dish_ingredient->assign_to_purchase_list( $list_id, $args{ucg} ); } } diff --git a/lib/Coocook/Schema/ResultSet/Item.pm b/lib/Coocook/Schema/ResultSet/Item.pm index 6a1948cec..a61aa74fd 100644 --- a/lib/Coocook/Schema/ResultSet/Item.pm +++ b/lib/Coocook/Schema/ResultSet/Item.pm @@ -5,26 +5,27 @@ use Coocook::Base qw(Moose); extends 'Coocook::Schema::ResultSet'; # find item and add value or create new item with value -sub add_or_create ( $self, $args ) { +sub add_or_create ( $self, %args ) { my $item = $self->find_or_new( { - purchase_list_id => $args->{purchase_list_id}, - article_id => $args->{article_id}, - unit_id => $args->{unit_id}, + purchase_list_id => $args{purchase_list_id}, + article_id => $args{article_id}, + unit_id => $args{unit_id}, } ); if ( $item->in_storage ) { - $item->value( $item->value + $args->{value} ); + $item->delta_to_value_or_recalculate( 0 => $args{value}, undef => $item->unit_id, $args{ucg} ); } else { $item->set_columns( { - value => $args->{value}, - comment => "", # no argument because existing items have comments - offset => 0, # instantly apply default_value + value => $args{value}, + comment => "", # no argument because existing items have comments + offset => 0, # instantly apply default_value } ); + return $item->insert(); } $item->update_or_insert and return $item; diff --git a/t/controller_Item.t b/t/controller_Item.t index ab65b726c..8325c8615 100644 --- a/t/controller_Item.t +++ b/t/controller_Item.t @@ -55,6 +55,24 @@ subtest "remove offset" => sub { }; subtest "convert item" => sub { + $t->post('/project/1/Test-Project/items/999/convert'); + $t->status_is(404); + + $t->post( '/project/1/Test-Project/items/1/convert', { unit => 2 } ); # 'total' missing + $t->status_is(400); + + $t->post( '/project/1/Test-Project/items/1/convert', { total => 42, unit => 999 } ); + $t->status_is(404); + + $t->post( '/project/1/Test-Project/items/1/convert', { total => 42, unit => 3 } ); + $t->status_is(400); + $t->text_contains("No conversion factor"); + + $t->post( '/project/1/Test-Project/items/1/convert', { total => 999, unit => 2 } ); + $t->status_is(400); + $t->text_contains("New total doesn’t fit to conversion"); + + $t->get_ok('/project/1/Test-Project/purchase_list/1'); $t->text_contains("14.5\N{THIN SPACE}kilograms"); $t->content_contains( my $old_value = 'value="14.5"' ); $t->content_lacks( my $new_value = 'value="14500"' ); diff --git a/t/schema_DishIngredient.t b/t/schema_DishIngredient.t index 9905b0d7b..4de9c047b 100644 --- a/t/schema_DishIngredient.t +++ b/t/schema_DishIngredient.t @@ -105,15 +105,15 @@ subtest "update ingredient value" => sub { "exception for negative value"; }; -subtest "update ingredient value and unit" => sub { +subtest set_value_unit_update_item => sub { my ( $g, $kg, $l, $p ) = map { $units->find( { short_name => $_ } )->id } qw( g kg l p ); - t "1g", sub { $_->set_value_unit_update_item( 2, $kg ) } => "2kg", "ingredient without item"; + t "1g", sub { !$_->set_value_unit_update_item( 2, $kg ) } => "2kg", "ingredient without item"; t "1+0kg: 1kg", sub { $_->set_value_unit_update_item( 500, $g ) } => "0.5+0kg: 500g", "basic item with conversion"; - t "1+0kg: 1kg", sub { $_->set_value_unit_update_item( 0.5, $l ) } => "0.5+0l: 0.5l", + t "1+0kg: 1kg", sub { !$_->set_value_unit_update_item( 0.5, $l ) } => "0.5+0l: 0.5l", "basic item without conversion"; t "2+0kg: 1kg 1000g", sub { $_->set_value_unit_update_item( 500, $g ) } => "1.5+0kg: 500g 1000g", @@ -123,8 +123,8 @@ subtest "update ingredient value and unit" => sub { sub { $_->set_value_unit_update_item( 0.5, $l ) } => [ "1+0kg: 1000g", "0.5+0l: 0.5l" ], "kg to liter"; - t "2+0kg: 1l 1000g", - sub { $_->set_value_unit_update_item( 500, $g ) } => [ "1+0kg: 1000g", "500+0g: 500g" ], + t "2+0kg: 1l 1000pcs", + sub { $_->set_value_unit_update_item( 500, $g ) } => [ "0.5+0kg: 500g", "1000+0pcs: 1000pcs" ], "liter to grams"; t "2+0kg: 1l 1000g", @@ -251,6 +251,10 @@ sub set_items_ingredients (@strings) { return @ingredients; } +# short function name for tests on dish ingredients: +# - sets dish ingredients to $input +# - executes $coderef on dish ingredient(s) which must return true +# - compares dish ingredients to $expected sub t ( $input, $coderef, $expected, $name = undef ) { local $Test::Builder::Level = $Test::Builder::Level + 1; diff --git a/t/schema_Item.t b/t/schema_Item.t index 94783231f..4ebbc32d5 100644 --- a/t/schema_Item.t +++ b/t/schema_Item.t @@ -4,12 +4,12 @@ use Test2::V0; use lib "t/lib"; use TestDB qw(txn_do_and_rollback); -plan( tests => 10 ); +plan( tests => 12 ); my $db = TestDB->new(); -# this is a SQL query useful debugging -# is it only run to assert that it matches the current schema +# this is a SQL query useful for debugging +# is it only run to assert that it works with the current schema $db->storage->dbh_do( sub ( $storage, $dbh ) { $dbh->do(<resultset('Project')->find(1); my $items = $project->purchase_lists->find(1)->items; my $ucg = $project->unit_conversion_graph(); -my $l = $project->units->find( { short_name => 'l' } ); -my $t = $project->units->find( { short_name => 't' } ); -{ +my ( $g, $kg, $t, $ml, $l ) = + map { $project->units->find( { short_name => $_ } ) } qw( g kg t ml l ); + +subtest delta_to_value_or_recalculate => sub { my $item = $items->find(1); # repeatedly a block variable to avoid caching in Result object + like dies { $item->delta_to_value_or_recalculate( undef, undef, undef, undef ) } => qr/value 1/i; + like dies { $item->delta_to_value_or_recalculate( 0, undef, undef, undef ) } => qr/value 2/i; + like dies { $item->delta_to_value_or_recalculate( 0, 0, undef, undef ) } => qr/unit 1 and 2/i; + like dies { $item->delta_to_value_or_recalculate( 1, 1, undef, 1 ) } => qr/unit for value 1/i; + like dies { $item->delta_to_value_or_recalculate( 1, 1, 1, undef ) } => qr/unit for value 2/i; + + # passing test cases without exception are tested in schema_DishIngredient.t +}; + +{ + my $item = $items->find(1); + like dies { $item->remove_ingredients() } => qr/arguments/, "remove_ingredients() without UnitConversionGraph"; @@ -113,16 +126,24 @@ subtest "remove l from kg (unit without conversion)", txn_do_and_rollback $db, s is $ingredient->item_id => undef; }; -subtest "handling of l (unit without conversion) during removal", txn_do_and_rollback $db, sub { - my $ingredient2 = $item->ingredients->find(1); - $ingredient2->update( { unit_id => $l->id } ); # set to l to force recalculation - $item->ingredients->update( { value => 0 } ); # value 0 must is valid, item must not be removed - is $item->remove_ingredients( $ucg, $ingredient2 ) => exact_ref($item); - $item->discard_changes(); +subtest "calculate_value_from_ingredients", txn_do_and_rollback $db, sub { + is $item->calculate_value_from_ingredients() => exact_ref($item); + is $item->ingredients->count => 4; + is $item->value => 14.5; + $ingredient->discard_changes(); - is $item->value => 0.0, "item value recalculated"; - isnt $ingredient->item_id => $item->id, "item_id of l ingredient changed"; - is $ingredient->item->ingredients->count => 1, "new item has only 1 ingredient"; + isnt $ingredient->item_id => $item->id, "new item ID"; + is $ingredient->item->value => 1, "value of 5th ingredient"; + is $ingredient->unit_id => $l->id, "same unit ID"; +}; + +subtest "calculate_value_from_ingredients", txn_do_and_rollback $db, sub { + $item->ingredients->find(1)->update( { unit_id => $l->id } ); # set to l to force recalculation + $item->ingredients->update( { value => 0 } ); # value 0 is valid, item must not be removed + is $item->calculate_value_from_ingredients() => exact_ref($item); + $item->discard_changes(); + is $item->value => 0.0, "item value recalculated"; + is $item->ingredients->count => $_, "item still has $_ ingredients" for 5; }; subtest "remove all ingredients", txn_do_and_rollback $db, sub { -- GitLab