Merge pull request #389 from karlomikus/develop

Update substitute counting
karlomikus · Dec 23, 2024 · 3a3b666 · 3a3b666
2 parents b56a85a + 5787455
commit 3a3b666
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,6 @@
 # v4.2.5
 ## Fixes
+- Fixed overcounting ingredients which led to incorrect recipe matching if multiple substitutes were used
 - Added missing filter `missing_bar_ingredients` attribute to `/cocktails` endpoint
 
 # v4.2.4

diff --git a/app/Repository/CocktailRepository.php b/app/Repository/CocktailRepository.php
@@ -50,13 +50,17 @@ public function getCocktailsByIngredients(array $ingredientIds, ?int $limit = nu
             $ingredientIds = array_unique($ingredientIds);
         }
 
+        // This query should handle the following cases:
+        // Correctly count one match when either the main ingredient OR any of its substitutes match
+        // Not overcount when multiple substitutes exist for the same ingredient
+        // If enabled, also match parent ingredients of the specified ingredients
+        // If an ingredient can be matched either directly or through a substitute, it should only count once
         $query = $this->db->table('cocktails')
             ->select('cocktails.id')
             ->selectRaw(
-                '
-                COUNT(DISTINCT CASE
+                'COUNT(DISTINCT CASE
                     WHEN ingredients.id IN (' . str_repeat('?,', count($ingredientIds) - 1) . '?) THEN ingredients.id
-                    WHEN cocktail_ingredient_substitutes.ingredient_id IN (' . str_repeat('?,', count($ingredientIds) - 1) . '?) THEN cocktail_ingredient_substitutes.ingredient_id
+                    WHEN cocktail_ingredient_substitutes.ingredient_id IN (' . str_repeat('?,', count($ingredientIds) - 1) . '?) THEN ingredients.id
                     WHEN ? = true AND ingredients.id IN (
                         SELECT parent_ingredient_id 
                         FROM ingredients 

diff --git a/tests/Feature/Repository/CocktailRepositoryTest.php b/tests/Feature/Repository/CocktailRepositoryTest.php
@@ -132,6 +132,53 @@ public function test_gets_cocktails_that_can_be_made_with_substitute_ingredients
         $this->assertSame([1], $cocktails->toArray());
     }
 
+    public function test_correctly_counts_substitutes_match(): void
+    {
+        $membership = $this->setupBarMembership();
+        $this->actingAs($membership->user);
+
+        $ingredient1 = Ingredient::factory()->for($membership->bar)->create();
+        $ingredient2 = Ingredient::factory()->for($membership->bar)->create();
+        $ingredient3 = Ingredient::factory()->for($membership->bar)->create();
+        $ingredient4 = Ingredient::factory()->for($membership->bar)->create();
+        $ingredient5 = Ingredient::factory()->for($membership->bar)->create();
+
+        Cocktail::factory()
+            ->for($membership->bar)
+            ->has(
+                CocktailIngredient::factory()
+                    ->state(['optional' => false])
+                    ->for($ingredient1)
+                    ->has(CocktailIngredientSubstitute::factory()->for($ingredient2), 'substitutes')
+                    ->has(CocktailIngredientSubstitute::factory()->for($ingredient5), 'substitutes')
+                    ->recycle($membership->bar),
+                'ingredients'
+            )
+            ->has(
+                CocktailIngredient::factory()
+                    ->state(['optional' => false])
+                    ->for($ingredient3)
+                    ->recycle($membership->bar),
+                'ingredients'
+            )
+            ->has(
+                CocktailIngredient::factory()
+                    ->state(['optional' => false])
+                    ->for($ingredient4)
+                    ->recycle($membership->bar),
+                'ingredients'
+            )
+            ->create();
+
+        Ingredient::factory()->for($membership->bar)->count(10)->create();
+        Cocktail::factory()->recycle($membership->bar)->count(10)->create();
+
+        $repository = resolve(CocktailRepository::class);
+        $cocktails = $repository->getCocktailsByIngredients([$ingredient2->id, $ingredient3->id, $ingredient5->id]);
+
+        $this->assertCount(0, $cocktails->toArray());
+    }
+
     public function test_gets_cocktails_that_can_be_made_with_complex_ingredients(): void
     {
         $membership = $this->setupBarMembership();