Fix Controller::Dish delete function

[moseschmiedel]

When deleting a dish, it's ingredient's item get also deleted.

Fixes #33

[dboehmer]

How could we have less code duplication? Obviously this logic should move to model code (Coocook::Model or Coocook::Schema namespace).

I'm still thinking about this. Some thoughts:

• probably a method of Schema::Result::DishIngredient
• with the Moose object system we could run code when update() or delete() is called and adjust the item accordingly—much comfort (good), much magic (bad)
• if a dedicated method, what name?
• 2-call-model: $ingredient->update(...); $ingredient->magic_with_item()?
• 1-call-model: $ingredient->update_with_item(...) or $ingredient->delete_with_item()?

[moseschmiedel]

$dish->update_items_and_delete Hereby @dboehmer, @markusleupold and @moseschmiedel decided, that the function name, which is written above is superior and should be immediately implemented.

[moseschmiedel]

A question, that came to my mind, while I was overthinking the implementation of the update and delete function was: What if we store all values of ingredients in the default unit of the quantity and just change the value for display to the desired unit. So internally we could just add and substract without keeping track of the current unit and the user can freely select in which unit he wants to see the value.

In my mind the implementation would have some function like value_in_unit (unit) which is part of Result::DishIngredient and which gives item the value in the desired unit. This would mean, that I could, when I want to convert an item into another unit, just ask every ingredient, which is part of that item, what it's value is in the new unit and sum them all up.

One problem I have with this idea, is that we would have to migrate a lot of data to the new format.

[MarkusLeupold]

That's a very interesting idea. Such an approach would make some operations easier (e.g. the one @moseschmiedel spoke of). But we have to bear in mind, that (almost?) every user expects some default units on different ingredients.

A user story

Let's say, Ute wants to use some salt in a cake. She adds this ingredient and specifies the amount in teaspoons. Now, Ute also wants milk to be a part of the dough—roughly 250 milliliters. The next day, Ute want's to bake her cake but doesn't remember everything. She looks it up and finds 0.25 liters of milk and 0.0005 liters of salt.

What happened there? Coocook saved the amounts internally in liters, due to these two amounts are volume measures and liters are a standard unit for volume.

While the first position is easy to understand, Ute has to guess, which unit is the best to convert 0.0005 liters to. Luckily, Ute can build upon her huge kitchen experience and knows that salt for a cake can normally be measured in teaspoons. She converts it using two clicks and gets a well-understandable one-and-a-half teaspoons of salt. Two more clicks aren't this difficult. But doing this with nearly every ingredient makes using Coocook awkward and exhausting. So next time Ute would probably turn back to pen and paper 'cause these two remember her favourite units. Now, imagine Tyler using Ute's recipe to impress his girlfriend (after someone has translated the recipe into English). He doesn't know much about typical amounts of salt or milk. So he has to switch through all the available units until he finds one that helps him. Let's hope that he finishes the cake before his girlfriend arrives...

Conclusion

As we see, different ingredients often have their own best-suited units. Using standard units in storage for each quantity means losing the information about the best-suited unit. We have to make up for that somewhere, somehow.

Thinking further: Wouldn't it also be nice if Coocook was even smarter and would choose the best unit automatically? For example, small amounts of liquid can be displayed in milliliters. But if we cook for a hundred people Coocook switches to liter-display automatically.

[moseschmiedel]

@MarkusLeupold I like your comment, I would then propose, that we still keep track of the unit, in which the user first created the Ingredient, but store the value in the default unit. Now when the recipe is viewed, Coocook automatically gives the value in the unit, that is specified in the unit property, respectively it just multiplies the intern value, which is e.g. in liter with the right factor to get the amount in the desired value, but it don't changes the value property.

Another example would be time: We can count time in a weird complex way with wrapping seconds, minutes and hours and keeping track of days, months and years or we just simply can count in seconds and then we want to display the time in an human readable format we just divide the seconds into years, months, days, hours, minutes and give back the rest as seconds.

[moseschmiedel]

I think, that the default unit should maybe already be set in the article, from which the ingredient gets created.

[dboehmer]

That's a very interesting idea. Such an approach would make some operations easier (e.g. the one @moseschmiedel spoke of). But we have to bear in mind, that (almost?) every user expects some default units on different ingredients.

...

You've used a quotation here for some reason although this is your own text, isn't it?

I see, you were right that we need documentation on the database schema for developers:-)

• each dish has value and unit and might have a (purchase list) item
• each item has 1 or more ingredients
• currently each item has a value and unit of its own + an offset for the value

The item value is expected to be the sum if its ingredients' values. This makes display easy.

We could drop the value column and compute the sum from all ingredients on every request.

@MarkusLeupold it's not as hard as you describe it: Only ingredients of the same article can be linked to the same item and only by explicitly converting one item's unit to another item's unit, e.g. converting 500g apples to 0.5kg apples to unify it with existing 42kg apples.

Open question: What happens with items merged from ingredients with different units when I change units later on? Not likely for kg but possible for tablespoons. Yesterday you converted x tbsp salt to grams, today someone changes the tbsp/g factor and now your purchase list contains a different number than before. Unlikely but very unexpected behavior.

I see little benefit in dropping the value column. You need to do conversion either while editing or while displaying. I'd rather chose while editing. Lower chance to miss any errors, no distant side effects.

Thinking further: Wouldn't it also be nice if Coocook was even smarter and would choose the best unit automatically? For example, small amounts of liquid can be displayed in milliliters. But if we cook for a hundred people Coocook switches to liter-display automatically.

Sounds good but what exactly is the best unit? I'd argue the original unit from the recipe is often the best unit. Fitness of other units often depends on which package sizes are available for sale and we can't know that.

[MarkusLeupold]

You've used a quotation here for some reason although this is your own text, isn't it?

Yes, I did. I had no idea how to seperate the user story visually from the rest. I changed it to a headline based approach since the former one is apparently missleading.

[MarkusLeupold]

@MarkusLeupold it's not as hard as you describe it: Only ingredients of the same article can be linked to the same item and only by explicitly converting one item's unit to another item's unit, e.g. converting 500g apples to 0.5kg apples to unify it with existing 42kg apples.

@dboehmer I'm sorry, but I don't understand the relationship to my comment. In fact, I didn't even think about shopping list items but only about recipes themselves. My concern is that if we stored all amounts of ingredients in the standard unit of the corresponding quantity (@moseschmiedel's idea), we would lose the information about the unit which the amount was originally specified in.

[dboehmer]

Just looking at the Git history graph: You based your changes in this PR on 66af1c2c220c0c0a9322a0db1450fa982ee07e3a but in master we have already merged your previous change ca4cb60507e68c192ad830ad8fc2c95594482d86. You’ll need to rebase your changes on the current master.

$ git l --graph origin/master origin/issue33 --decorate=short | head -n10
* 94ace86 (HEAD, origin/issue33) Put business logic at better location.
* 03fe2c7 Fix Controller::Dish delete function When deleting a dish, it's ingredient's item get also deleted
| *   277e581 (origin/master, master) Merge branch 'issue110'
| |\  
|/ /  
| * ca4cb60 (orlando/issue110, origin/issue110) Fix dish update logic
|/  
* 66af1c2 Fix exception in dish editor on update for ingredients w/o item
* 483d414 Fix wording and add hint to README.md
* b86e52f Fix value of items when changing/deleting dish ingredients

[moseschmiedel]

Oh, sry. These changes were not meant to be final. I just committed them because I wanted to try to create a Dockerimage.