karlomikus / bar-assistant

Bar assistant is a all-in-one solution for managing your home bar
https://barassistant.app
MIT License
592 stars 22 forks source link

Cocktails whose missing ingredients can be made don't show up under 'Cocktails you can make' #313

Closed seamuslowry closed 3 months ago

seamuslowry commented 3 months ago

Describe the bug When I filter by "Cocktails I Can Make," cocktails for which the ingredients are either on the shelf or are "Missing, but you can make it with ingredients in your shelf" do not appear in the list.

To Reproduce

  1. Create ingredient 'Lemon'
  2. Add ingredient 'Lemon' to shelf
  3. Create ingredient 'Lemon Juice' where it contains 'Lemon' (indicating that, if you have 'Lemon' you can make 'Lemon Juice')
  4. Ensure 'Lemon Juice' is not on your shelf
  5. Create a cocktail with 'Lemon Juice' as an ingredient
  6. Ensure all other ingredients for that cocktail are on your shelf
  7. See that the cocktail does not appear under 'Cocktails I Can Make'

Versions:

Additional context, log outputs... No log outputs. I can see in the response for the cocktail, the 'Lemon Juice' ingredient configured as described is flagged as in_shelf: false (which is correct) and in_shelf_as_complex_ingredient: true (which is also correct). I believe that cocktails where all ingredients have some variation of in_shelf* property as true should show up under the 'Cocktails I Can Make' search.

karlomikus commented 3 months ago

Can you double check this, because with the minimal setup this works for me.

Ingredient setup: Snimka zaslona 2024-08-18 192137 Cocktail filters: Snimka zaslona 2024-08-18 192119

seamuslowry commented 3 months ago

I do still see the same behavior.

Trying the same setup, I'm still seeing the same behavior: image

image

Versions for everything: image

I'm on the barassistant/server:v3 docker tag. I see there's a dev tag pushed more recently. Is it possible the fix is unreleased? I'm not sure about the risks/drawbacks of using the dev tag on this project, so I haven't tried using that tag.

karlomikus commented 3 months ago

No, it's live since 3.17.

I'll try to write some SQL queries for you to run so we can debug this.

seamuslowry commented 3 months ago

~Appreciate it. My database is a postgres docker container on postgres:16 if that makes a difference~

This is wrong; sorry.

karlomikus commented 3 months ago

Well, that's the issue probably. The only "officially" supported database is sqlite.

While the migrations are designed to be database-agnostic, and the application might function with other databases, more complex queries can yield different results depending on the database in use. That's why I don't want to test/support a lot of different db engines.

seamuslowry commented 3 months ago

Well, that's the issue probably. The only "officially" supported database is sqlite.

That's my bad. I started hosting a few new services recently and got my wires crossed on this. I'm using the docker image and not doing anything special to try to change the database; it should be using sqlite.

seamuslowry commented 3 months ago

I think one or both of my 'Lemon' and 'Lemon Juice' were from seeded data. So I tried recreating them to see if that resolved the issue. It did not. I'm not sure what else to test to determine what the issue is. The database is whatever the default from the docker image is.

karlomikus commented 3 months ago

Can you share what are the results of the following commands.

docker compose exec bar-assistant sqlite3 storage/bar-assistant/database.ba3.sqlite "select * from ingredients as i join user_ingredients as ui on ui.ingredient_id = i.id where i.name like '%lemon%';"
docker compose exec bar-assistant sqlite3 storage/bar-assistant/database.ba3.sqlite "select ci.* from complex_ingredients as ci join ingredients as i on ci.ingredient_id = i.id where i.name like '%lemon%';"
docker compose exec bar-assistant sqlite3 storage/bar-assistant/database.ba3.sqlite "select ci.* from cocktails as c join cocktail_ingredients as ci on ci.cocktail_id = c.id where c.name like '%test%';"

Or if you can, share your database file, should be in your mounted folder database.ba3.sqlite. You can send it to contact@karlomikus.com.

seamuslowry commented 3 months ago

I can do all of the above

docker compose exec bar-assistant sqlite3 storage/bar-assistant/database.ba3.sqlite "select * from ingredients as i join user_ingredients as ui on ui.ingredient_id = i.id where i.name like '%lemon%';"

295|2|lemon-2|Lemon|0|Lemon is a citrus fruit known for its bright yellow color and tart flavor.||#fdf26f|19||1|1|2024-08-11 19:02:30|2024-08-17 20:57:31|34|2|295
294|2|lemon-juice-2|Lemon juice|0|Freshly squeezed lemon juice.||#f3efda|18||1|1|2024-08-11 19:02:30|2024-08-24 11:33:28|36|2|294

docker compose exec bar-assistant sqlite3 storage/bar-assistant/database.ba3.sqlite "select ci.* from complex_ingredients as ci join ingredients as i on ci.ingredient_id = i.id where i.name like '%lemon%';"

9|294|295

docker compose exec bar-assistant sqlite3 storage/bar-assistant/database.ba3.sqlite "select ci.* from cocktails as c join cocktail_ingredients as ci on ci.cocktail_id = c.id where c.name like '%test%';"

2593|294|609|1||oz|1|0|

Additionally, the SQLite file is sent to that email address.

karlomikus commented 3 months ago

Took a while to debug, but seems like there indeed is an issues with matching logic. It depends on number of ingredients in the shelf or something, don't really know since that matching is a bit complex.

I have improved query ready, but I need to test it a bit more.

seamuslowry commented 3 months ago

Bless. Thanks for looking into it.

karlomikus commented 3 months ago

Should be fixed with newest release

seamuslowry commented 3 months ago

The test and real cocktails now show up as expected under 'Cocktails you can make'! The only odd behavior is that the recommended ingredients section of the home page still includes the ingredients I can make:

image

I'm not sure what the intended behavior is for that section. I could see it being reasonable either way, but I would've expected that 'Lemon juice' and 'Lime juice' would not show up in the section since I had lemons and limes.

karlomikus commented 3 months ago

That's a good point. I think this should also take complex ingredients in the calculation.