diff --git a/lib/Coocook/Controller/Ajax/IngredientsEditor.pm b/lib/Coocook/Controller/Ajax/IngredientsEditor.pm index e5173f0a96794a2d9c415862d668dd588a1aebe2..4637c5dc7c2c52b7e9aa0a6003c4538624d3c362 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 8dc821d757bddfcc9d123e1f3b11500d0dbce7fb..2c146308995f52e7f8b9e8f09bdb32813d7324bc 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 modified ."] ); } - 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 7f901082e7a91d8a68351195efd3c9d70dba2fce..983017747d12ce1c636fe0bcb712da75bd6b1bf2 100644 --- a/lib/Coocook/Schema/Result/DishIngredient.pm +++ b/lib/Coocook/Schema/Result/DishIngredient.pm @@ -63,7 +63,7 @@ __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( @@ -74,6 +74,7 @@ sub assign_to_purchase_list ( $self, $list_id ) { article_id => $self->article_id, unit_id => $self->unit_id, value => $self->value, + ucg => $ucg, } ); @@ -86,97 +87,69 @@ 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 } ); - - my $purchase_list_id = $item->purchase_list_id; + $self->delete(); - if ( $item->ingredients->count == 0 ) { # TODO use results_exist() - $item->delete(); - $item = undef; + 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 ); } - if ( $new_value > 0 ) { - return $self->assign_to_purchase_list($purchase_list_id); - } - 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; + } + ); +} + +=head2 set_value_update_item( $value, $ucg? ) - $ucg ||= $item->purchase_list->project->unit_conversion_graph; +Returns the updated C or +an empty list if it didn’t exist or was deleted. - 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 ); - } - } +=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; + 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; + 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 e80c8d2d29544b7c42d123e77033b9d68e29dd9e..388fab70de9f0aff3cb83ec577652eb06335f9bc 100644 --- a/lib/Coocook/Schema/Result/Item.pm +++ b/lib/Coocook/Schema/Result/Item.pm @@ -61,97 +61,105 @@ __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 delta 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 ); -} - -=head2 set_value_offset($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"; -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. + if ( ( $value1 and $unit1_id != $self->unit_id ) + or ( $value2 and $unit2_id != $self->unit_id ) ) + { + $ucg ||= $self->purchase_list->project->unit_conversion_graph; -=cut + my $factor1 = + ( $value1 and $unit1_id != $self->unit_id ) + ? $ucg->factor_between_units( $unit1_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 ); -} + my $factor2 = + ( $value2 and $unit2_id != $self->unit_id ) + ? $ucg->factor_between_units( $unit2_id => $self->unit_id ) + : 1; -sub _set_value_offset ( $self, $delta, $value ) { # TODO order of methods in this file - $self->set_column( value => $value ); + if ( !$factor1 or !$factor2 ) { + my $factor1_2 = + ( $unit1_id and $unit2_id ) ? $ucg->factor_between_units( $unit1_id => $unit2_id ) : undef; - my $offset = $self->offset; + if ( $factor1_2 and $value1 * $factor1_2 == $value2 ) { + return $self; # value1/2 cannot be converted 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(); # might remove item } + + $value1 *= $factor1; + $value2 *= $factor2; } - $self->update( { offset => 0 } ); # otherwise clear offset + $self->_delta_to_value_adjust_offset( $value2 - $value1 ); + return $self; } -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 if 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( - { - purchase_list_id => $self->purchase_list_id, - article_id => $self->article_id, - unit_id => $unit_id, - } - ); - - if ($existing_item) { - $existing_item->update( + if ( $unit_id != $self->unit_id ) { + my $existing_item = $self->result_source->resultset->find( { - value => $existing_item->value + $value, - offset => $existing_item->offset + $offset, + purchase_list_id => $self->purchase_list_id, + article_id => $self->article_id, + unit_id => $unit_id, } ); - $self->ingredients->update( { item_id => $existing_item->id } ); - - $self->delete; + if ($existing_item) { + $existing_item->update( + { + value => $existing_item->value + $value, + offset => $existing_item->offset + $offset, + } + ); - return $existing_item; + $self->ingredients->update( { item_id => $existing_item->id } ); + $self->delete; + return $existing_item; + } } - else { - $self->update( - { - unit_id => $unit_id, - value => $value, - offset => $offset, - } - ); - return $self; - } + $self->update( + { + unit_id => $unit_id, + value => $value, + offset => $offset, + } + ); + + return $self; } ); } @@ -159,6 +167,7 @@ sub change_value_offset_unit ( $self, $value, $offset, $unit_id ) { =head2 remove_ingredients( $union_conversion_graph, @ingredient_results ) Returns the updated C or C if it has been deleted. +The dish ingredients are detached from the item but B deleted. =cut @@ -166,7 +175,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 +184,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 +222,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 $remaining_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_item_value += $ingredient->value * $factor; + $remaining_ingredients++; next; } @@ -281,32 +257,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 > 0 ) { + $self->update( { value => $remaining_item_value, offset => 0 } ); + return $self; + } + else { $self->delete(); return; } + } + ); +} - my @items; +=head2 _delta_to_value_adjust_offset($delta) - # 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; +Adds/subtracts C<$delta> to/from the item’s value and adjusts the item’s offset. +The delta must be given in the item’s unit. +Does B change any dish ingredients. - } - } +=cut - my $item = $ingredient->assign_to_purchase_list( $self->purchase_list_id ); - push @items, $item; - } +sub _delta_to_value_adjust_offset ( $self, $delta ) { + return $self if $delta == 0; - $self->update( { value => $item_value, offset => 0 } ); - return $self; + my $value = $self->value + $delta; + $value >= 0 or croak "Delta creates negative value"; + + $self->set_column( value => $value ); + + 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/Project.pm b/lib/Coocook/Schema/Result/Project.pm index 7776746b88f9ba52255114069a9407950084e69e..8fb591a1efadaa6ff84605c4fff9062cb478053f 100644 --- a/lib/Coocook/Schema/Result/Project.pm +++ b/lib/Coocook/Schema/Result/Project.pm @@ -136,6 +136,41 @@ sub articles_cached_units ($self) { } } +=head2 assign_unassigned_dish_ingredients() + +Assign dish ingredients with no C to a new purchase list +created on demand with the name I. +If there is a purchase list with that name already, the name +is suffixed with the lowest integer possible to create a unique name. + +Returns the C object if one was created. + +=cut + +sub assign_unassigned_dish_ingredients ($self) { + my $unassigned_ingredients = $self->dish_ingredients->unassigned; + + $self->purchase_lists->results_exist or return; + $unassigned_ingredients->results_exist or return; + + my $i = 1; + my $name = sub { "Previously unassigned dish ingredients" . ( $i > 1 ? " ($i)" : "" ) }; + while ( $self->purchase_lists->results_exist( { name => $name->() } ) ) { $i++ } + + my $list = $self->create_related( + purchase_lists => { + date => $self->purchase_lists->default_date, + name => $name->(), + } + ); + + while ( my $ingredient = $unassigned_ingredients->next ) { + $ingredient->assign_to_purchase_list( $list->id ); + } + + return $list; +} + =head2 dish_ingredients() Shortcut. @@ -159,19 +194,15 @@ sub inventory ($self) { undef, { columns => { - articles => $self_rs->search_related('articles')->count_rs->as_query, - dishes => $self_rs->search_related('meals')->search_related('dishes')->count_rs->as_query, - meals => $self_rs->search_related('meals')->count_rs->as_query, - purchase_lists => $self_rs->search_related('purchase_lists')->count_rs->as_query, - recipes => $self_rs->search_related('recipes')->count_rs->as_query, - shop_sections => $self_rs->search_related('shop_sections')->count_rs->as_query, - tags => $self_rs->search_related('tags')->count_rs->as_query, - tag_groups => $self_rs->search_related('tag_groups')->count_rs->as_query, - units => $self_rs->search_related('units')->count_rs->as_query, - unassigned_items => $self_rs->search_related('meals') - ->search_related('dishes') - ->search_related('ingredients') - ->unassigned->count_rs->as_query, + articles => $self_rs->search_related('articles')->count_rs->as_query, + dishes => $self_rs->search_related('meals')->search_related('dishes')->count_rs->as_query, + meals => $self_rs->search_related('meals')->count_rs->as_query, + purchase_lists => $self_rs->search_related('purchase_lists')->count_rs->as_query, + recipes => $self_rs->search_related('recipes')->count_rs->as_query, + shop_sections => $self_rs->search_related('shop_sections')->count_rs->as_query, + tags => $self_rs->search_related('tags')->count_rs->as_query, + tag_groups => $self_rs->search_related('tag_groups')->count_rs->as_query, + units => $self_rs->search_related('units')->count_rs->as_query, }, } )->hri->single; diff --git a/lib/Coocook/Schema/Result/PurchaseList.pm b/lib/Coocook/Schema/Result/PurchaseList.pm index 3052f4bcd13ca0b1b597152eae3ff4fb6fb91819..2e44629b98d59839a6455f44bc8bfa6be24edbea 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 22797692e0616f2ae0a8398efbf5eedf40dab1af..640551522c6d765945e293f69b16fcd7c67147d9 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 6a1948cecc17e252ee5ce5be185b41269dbcb4e8..723641a397cd9e5453b3125fcdef4b8694bb1103 100644 --- a/lib/Coocook/Schema/ResultSet/Item.pm +++ b/lib/Coocook/Schema/ResultSet/Item.pm @@ -4,7 +4,26 @@ use Coocook::Base qw(Moose); extends 'Coocook::Schema::ResultSet'; -# find item and add value or create new item with value +=head2 add_or_create( { ... } ) + +Find item with given unit and add value +or create new item with given value. +Syntax with hashref like e.g. C from L. +Returns found or created C. + + # add 42 kg of apples to existing or new item on purchase list + my $item = $purchase_list->search_related('items')->add_or_create( + { + purchase_list_id => $purchase_list->id, + value => 42, + unit_id => $kg->id, + article_id => $apples->id, + ucg => $project->ucg, # optional + } + ); + +=cut + sub add_or_create ( $self, $args ) { my $item = $self->find_or_new( { @@ -15,19 +34,20 @@ sub add_or_create ( $self, $args ) { ); 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} ) + or die "found item with unit but delta_to_value_or_recalculate() returned undef"; + + return $item; } else { $item->set_columns( { value => $args->{value}, - comment => "", # no argument because existing items have comments - offset => 0, # instantly apply default_value + comment => "", # not an argument because existing items might already have comments } ); + return $item->insert(); } - - $item->update_or_insert and return $item; } __PACKAGE__->meta->make_immutable; diff --git a/lib/Coocook/Schema/ResultSet/Project.pm b/lib/Coocook/Schema/ResultSet/Project.pm index 7224d30e71737f29ad6d6d31037eaa32804cf722..83e444b368d5f0657ce4f292eecf793684f307e1 100644 --- a/lib/Coocook/Schema/ResultSet/Project.pm +++ b/lib/Coocook/Schema/ResultSet/Project.pm @@ -40,4 +40,40 @@ sub stale ( $self, $pivot_date = undef ) { { -not_bool => [ map { -exists => $_->search( undef, { select => [ \1 ] } )->as_query } @rs ] } ); } +=head2 with_unassigned_dish_ingredients() + +Returns a new resultset with projects +that have dish ingredients and purchase list +but some dish ingredient is not assigned to any purchase list. + +The result objects returned from the resultset will have +the additional column C. + + my $invalid_projects = $projects->with_unassigned_dish_ingredients(); + print $invalid_projects->first->get_column('unassigned_dish_ingredients_count'); + +=cut + +sub with_unassigned_dish_ingredients ($self) { + return $self->search( + { + -and => [ + $self->correlate('purchase_lists')->results_exist_as_query, + $self->correlate('meals') + ->search_related('dishes') + ->search_related('ingredients') + ->unassigned->results_exist_as_query, + ] + }, + { + '+columns' => { + unassigned_dish_ingredients_count => $self->correlate('meals') + ->search_related('dishes') + ->search_related('ingredients') + ->unassigned->count_rs->as_query, + }, + } + ); +} + 1; diff --git a/lib/Coocook/Script/Dbck.pm b/lib/Coocook/Script/Dbck.pm index 4bb5830fb323b8d18731d1b5904ed044706e51b0..5f86703865cf1dc2cdbee47316ce9716d06618d6 100644 --- a/lib/Coocook/Script/Dbck.pm +++ b/lib/Coocook/Script/Dbck.pm @@ -319,30 +319,17 @@ sub check_missing_default_purchase_lists ($self) { } sub check_unassigned_dish_ingredients ($self) { - my $projects = $self->_schema->resultset('Project'); - my $invalid_projects = $projects->search( - { - -and => [ - $projects->correlate('purchase_lists')->results_exist_as_query, - $projects->correlate('meals') - ->search_related('dishes') - ->search_related('ingredients') - ->unassigned->results_exist_as_query, - ] - }, - { - '+columns' => { - unassigned_items_count => $projects->correlate('meals') - ->search_related('dishes') - ->search_related('ingredients') - ->unassigned->count_rs->as_query, - }, - } - ); + my $invalid_projects = $self->_schema->resultset('Project')->with_unassigned_dish_ingredients; while ( my $project = $invalid_projects->next ) { warn sprintf "Project %i has purchase list(s) but also %i unassigned dish ingredient(s)\n", - $project->id, $project->get_column('unassigned_items_count'); + $project->id, $project->get_column('unassigned_dish_ingredients_count'); + + $self->fix or next; + + $project->assign_unassigned_dish_ingredients(); + + warn "... Fixed!\n"; } } diff --git a/root/templates/project/show.tt b/root/templates/project/show.tt index cc97c5bbf55d641f079ee0515516e7c067bf76b5..7682e0811a5504c46d276a2b9c07dc78e4a6969d 100644 --- a/root/templates/project/show.tt +++ b/root/templates/project/show.tt @@ -41,9 +41,6 @@ show_donate_infobox = 0 %] [% END %] [% ELSIF inventory.purchase_lists == 0 %] This project lacks any purchase lists. You can create some. - [% ELSIF inventory.unassigned_items > 0 %] - There are some ingredient items not yet assigned to a purchase list. - You can assign items to purchase lists now. [% ELSIF project.is_stale; show_donate_infobox = 1 %]
diff --git a/share/ddl/_common/upgrade/27-28/unassigned_dish_ingredients.pl b/share/ddl/_common/upgrade/27-28/unassigned_dish_ingredients.pl index 784e81e85799dba3c93272789192a9da0dc421c9..972d7f421a5700843bbe03c41b80384fd440a12e 100644 --- a/share/ddl/_common/upgrade/27-28/unassigned_dish_ingredients.pl +++ b/share/ddl/_common/upgrade/27-28/unassigned_dish_ingredients.pl @@ -1,27 +1,9 @@ use Coocook::Base; sub ( $schema, $versions ) { - my $projects = $schema->resultset('Project'); + my $projects = $schema->resultset('Project')->with_unassigned_dish_ingredients; while ( my $project = $projects->next ) { - my $unassigned_ingredients = $project->dish_ingredients->unassigned; - - $project->purchase_lists->results_exist or next; - $unassigned_ingredients->results_exist or next; - - my $i = 1; - my $name = sub { "Previously unassigned items" . ( $i > 1 ? " ($i)" : "" ) }; - while ( $project->purchase_lists->find( { name => $name->() } ) ) { $i++ } - - my $list = $project->create_related( - purchase_lists => { - date => $project->purchase_lists->default_date, - name => $name->(), - } - ); - - while ( my $ingredient = $unassigned_ingredients->next ) { - $ingredient->assign_to_purchase_list( $list->id ); - } + $project->assign_unassigned_dish_ingredients(); } }; diff --git a/t/controller_Item.t b/t/controller_Item.t index ab65b726c9e7c47b1060647285b90baa1aa73421..8325c86152e5d2fe4e149699235f5cb2ad9680b7 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/postgresql.t b/t/postgresql.t index 12b5843e38a3d7d81e1f39bdcb15a3b63f1ae966..457d651e9a85fd2848e52573b27f468263531bec 100644 --- a/t/postgresql.t +++ b/t/postgresql.t @@ -329,7 +329,7 @@ subtest "migration 24->25 (issue #266 order of meals/dishes)" => sub { "dishes in order of insertion into database"; }; -subtest "migration 27->28 (issue #292 unassigned items to purchase list)" => sub { +subtest "migration 27->28 (issue #292 unassigned dish ingredients to purchase list)" => sub { my $schema = TestDB->new( deploy => 0 ); install_ok $schema, 24; # latest schema version that works with v21_install.sql ok TestDB->execute_test_data( $schema, 't/test_data_v21_install.sql' ), @@ -344,8 +344,8 @@ subtest "migration 27->28 (issue #292 unassigned items to purchase list)" => sub $purchase_lists->populate( [ [ 'date', 'name' ], - map { [ $purchase_lists->default_date, $_ ] } 'Previously unassigned items', - 'Previously unassigned items (2)' + map { [ $purchase_lists->default_date, $_ ] } 'Previously unassigned dish ingredients', + 'Previously unassigned dish ingredients (2)' ] ); @@ -371,7 +371,8 @@ subtest "migration 27->28 (issue #292 unassigned items to purchase list)" => sub is $project1->dish_ingredients->unassigned->count => 0, "project 1 has no more unassigned dish ingredients"; - ok $schema->resultset('PurchaseList')->find( { name => "Previously unassigned items (3)" } ), + ok $schema->resultset('PurchaseList') + ->find( { name => "Previously unassigned dish ingredients (3)" } ), "created purchase list"; is $project2->purchase_lists->count => 0, diff --git a/t/schema_DishIngredient.t b/t/schema_DishIngredient.t index 9905b0d7b54ddcc4fd7e3288fa13571c3bbca8f7..4de9c047b06bcf0b4f54d6c4ab145aabfdd01ae0 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 94783231f8495c5489d09ed3c9c760e51ab0152e..2a7c47bd4a6b9a0690364977b2e1be1d0ac7dcb8 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 => 13 ); 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 +# it is 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; + + # test cases without exception but actual results are tested in schema_DishIngredient.t +}; + +subtest "set_value_offset_unit()", txn_do_and_rollback $db, sub { + my $item = $items->find(2); + my $value = $item->value; + my $offset = $item->offset || die "want to test with offset"; + my $unit_id = $item->unit_id; + my $ingredients = $item->ingredients->count; + + subtest "with unchanged values" => sub { + is $item->set_value_offset_unit( $value, $offset, $unit_id ) => exact_ref($item); + $item->discard_changes(); + is $item => object { + call value => $value; + call offset => $offset; + call unit_id => $unit_id; + call sub { shift->ingredients->count } => $ingredients; + }; + }; + + subtest "with same unit but different values" => sub { + is $item->set_value_offset_unit( 90, 10, $unit_id ) => exact_ref($item); + $item->discard_changes(); + is $item => object { + call value => 90; + call offset => 10; + call unit_id => $unit_id; + call sub { shift->ingredients->count } => $ingredients; + }; + }; + + subtest "with different values and previously unused unit" => sub { + is $item->set_value_offset_unit( 900, 100, $l->id ) => exact_ref($item); + $item->discard_changes(); + is $item => object { + call value => 900; + call offset => 100; + call unit_id => $l->id; + call sub { shift->ingredients->count } => $ingredients; + }; + }; + + subtest "merge into existing item" => sub { + my $other_item = $items->create( + { + value => 42, + offset => 23, + unit_id => 1, + article_id => $item->article_id, + comment => __FILE__, + } + ); + $item->unit_id != $other_item->unit_id or die "test broken"; + + isnt my $new_item = + $item->set_value_offset_unit( 900, 100, $other_item->unit_id ) => exact_ref($item); + + $item->discard_changes(); + ok !$item->in_storage, "old item has been deleted"; + + $new_item->discard_changes(); + is $new_item => object { + prop isa => ref $item; + call in_storage => T(); + call id => $other_item->id; + call value => 900 + 42; + call offset => 100 + 23; + call unit_id => $other_item->unit_id; + call sub { shift->ingredients->count } => $ingredients; + }; + }; + +}; + +{ + my $item = $items->find(1); + like dies { $item->remove_ingredients() } => qr/arguments/, "remove_ingredients() without UnitConversionGraph"; @@ -43,7 +128,7 @@ my $t = $project->units->find( { short_name => 't' } ); is $item->value => 14.5, "... value unchanged"; } -subtest "remove 1kg", txn_do_and_rollback $db, sub { +subtest "remove ingredient with same unit: 1kg from kg", txn_do_and_rollback $db, sub { my $item = $items->find(1); my $ingredient = $item->ingredients->find(7); is $item->remove_ingredients( $ucg, $ingredient ) => exact_ref($item); @@ -53,7 +138,7 @@ subtest "remove 1kg", txn_do_and_rollback $db, sub { is $ingredient->item_id => undef; }; -subtest "remove 500g", txn_do_and_rollback $db, sub { +subtest "remove ingredient with direct conversion: 500g from kg", txn_do_and_rollback $db, sub { my $item = $items->find(1); my $ingredient = $item->ingredients->find(1); is $item->remove_ingredients( $ucg, $ingredient ) => exact_ref($item); @@ -63,7 +148,7 @@ subtest "remove 500g", txn_do_and_rollback $db, sub { is $ingredient->item_id => undef; }; -subtest "remove g from t", txn_do_and_rollback $db, sub { +subtest "remove ingredient with indirect conversion: g from t", txn_do_and_rollback $db, sub { my $item = $items->find(2); my $ingredient = $item->ingredients->find(2); @@ -105,7 +190,7 @@ my $ingredient = $item->ingredients->create( } ); -subtest "remove l from kg (unit without conversion)", txn_do_and_rollback $db, sub { +subtest "remove unit without conversion: l from kg", txn_do_and_rollback $db, sub { is $item->remove_ingredients( $ucg, $ingredient ) => exact_ref($item); $item->discard_changes(); $ingredient->discard_changes(); @@ -113,16 +198,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() with zero value", 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 { @@ -132,6 +225,7 @@ subtest "remove all ingredients", txn_do_and_rollback $db, sub { ok !$item->in_storage, "item was deleted"; for (@ingredients) { $_->discard_changes(); + ok $_->in_storage; is $_->item_id => undef; } }; diff --git a/t/schema_Project.t b/t/schema_Project.t index c4e483d9c7be5628335bea0115bacb1a2b138f67..949a18a1dc7ad529bb91a0837680fae74db2f1c2 100644 --- a/t/schema_Project.t +++ b/t/schema_Project.t @@ -14,16 +14,15 @@ subtest inventory => sub { my $inventory = $db->resultset('Project')->find(1)->inventory(); is $inventory => { - articles => 5, - dishes => 4, - meals => 4, - purchase_lists => 2, - recipes => 1, - shop_sections => 2, - tags => 3, - tag_groups => 1, - units => 8, - unassigned_items => 0, + articles => 5, + dishes => 4, + meals => 4, + purchase_lists => 2, + recipes => 1, + shop_sections => 2, + tags => 3, + tag_groups => 1, + units => 8, }; }; diff --git a/t/script_dbck.t b/t/script_dbck.t index 5fe9534199ace3f201ac5e8b1817bf2b38b7be04..1037ec5c2cf1f784b45b036a13c0ab7453c68be1 100644 --- a/t/script_dbck.t +++ b/t/script_dbck.t @@ -224,7 +224,7 @@ subtest "project with purchase list but unassigned dish ingredients", txn_do_and "SQL query returns empty"; note "Creating another dish ingredient without assigning it ..."; - $db->resultset('DishIngredient')->create( + my $ingredient = $db->resultset('DishIngredient')->create( { dish_id => 1, prepare => 0, @@ -235,11 +235,20 @@ subtest "project with purchase list but unassigned dish ingredients", txn_do_and } ); - like warning { $app->run } => qr/unassigned/, "warns"; + is warning { $app->run } => + "Project 1 has purchase list(s) but also 1 unassigned dish ingredient(s)\n", + "warns"; is $db->storage->dbh_do( sub ( $storage, $dbh ) { $dbh->selectall_arrayref($sql) } ) => array { item T(); etc() }, "SQL query with relevant data"; + $app->fix(1); + is warnings { $app->run() } => + [ "Project 1 has purchase list(s) but also 1 unassigned dish ingredient(s)\n", "... Fixed!\n" ], + "with --fix"; + ok !warns { $app->run() }, "doesn't warn anymore"; + $app->fix(0); + note "Deleting all purchase lists ..."; $db->resultset('Project')->update( { default_purchase_list_id => undef } ); $db->resultset('PurchaseList')->delete(); diff --git a/t/sqlite.t b/t/sqlite.t index 7155fc13227820fb4479d083f381027822fec7ed..3de3a1c7aa0bb073f2a6d495ac0a58dfb8e6d5eb 100644 --- a/t/sqlite.t +++ b/t/sqlite.t @@ -129,7 +129,7 @@ subtest "migration 24->25 (issue #266 order of meals/dishes)" => sub { "dishes in order of insertion into database"; }; -subtest "migration 27->28 (issue #292 unassigned items to purchase list)" => sub { +subtest "migration 27->28 (issue #292 unassigned dish ingredients to purchase list)" => sub { my $schema = TestDB->new( deploy => 0 ); install_ok $schema, 24; # latest schema version that works with v21_install.sql ok TestDB->execute_test_data( $schema, 't/test_data_v21_install.sql' ), @@ -142,7 +142,9 @@ subtest "migration 27->28 (issue #292 unassigned items to purchase list)" => sub is $schema->resultset('DishIngredient')->unassigned->count => 0, "has no more unassigned dish ingredients"; - ok $schema->resultset('PurchaseList')->search( { name => "Previously unassigned items" } )->count, + ok $schema->resultset('PurchaseList') + ->search( { name => "Previously unassigned dish ingredients" } ) + ->count, "created purchase lists"; };