Fix #35: Prevent duplicate products in Fastest Delivery sort #99
Conversation
- Add .distinct(Product.id) to delivery_fastest sort queries - Prevents duplicate rows when products have multiple delivery options - Added comprehensive tests for both general and category-filtered cases - All 53 backend tests pass - All 34 E2E tests pass
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR attempts to fix issue #35 by preventing duplicate products when they have multiple delivery options. It adds .distinct(Product.id) to three query branches that join with delivery options, and includes comprehensive tests to verify the fix.
Critical Issue
The implementation has a fundamental bug with PostgreSQL's DISTINCT ON clause. When using .distinct(expression), the ORDER BY clause must start with the same expression. All three additions violate this rule by using distinct(Product.id) followed by ordering starting with DeliveryOption.estimated_days_min, which will cause database errors.
The fix requires restructuring the ORDER BY clauses to place Product.id first in all three locations.
| stmt = ( | ||
| stmt.join(ProductDeliveryLink) | ||
| .join(DeliveryOption) | ||
| .distinct(cast(Any, Product.id)) |
There was a problem hiding this comment.
Critical Bug: Same DISTINCT ON issue as lines 228 and 240. The distinct(Product.id) must be followed by order_by(Product.id, ...) as the first ordering column.
| stmt = ( | ||
| stmt.join(ProductDeliveryLink) | ||
| .join(DeliveryOption) | ||
| .distinct(cast(Any, Product.id)) |
There was a problem hiding this comment.
Critical Bug: When using DISTINCT ON with PostgreSQL, the expression in distinct() must match the first column(s) in order_by(). Here, distinct(Product.id) is used but order_by() starts with DeliveryOption.estimated_days_min. This will cause a database error.
The correct pattern is:
.distinct(Product.id)
.order_by(Product.id, DeliveryOption.estimated_days_min.asc(), ...)This same issue appears in all three locations where .distinct() was added (lines 228, 240, 254).
| stmt = ( | ||
| stmt.join(ProductDeliveryLink) | ||
| .join(DeliveryOption) | ||
| .distinct(cast(Any, Product.id)) |
There was a problem hiding this comment.
Critical Bug: Same DISTINCT ON issue as line 228. The distinct(Product.id) must be followed by order_by(Product.id, ...) as the first ordering column.
Description
Fixes duplicate items appearing when using the 'Fastest Delivery' sort option.
Root Cause
When joining ProductDeliveryLink and DeliveryOption tables without using DISTINCT, products with multiple delivery options appeared multiple times in the result set (once for each delivery option).
Solution
Added .distinct(Product.id) to all delivery_fastest sort queries to ensure each product appears only once in results.
Changes
Testing
✅ All 53 backend tests pass
✅ All 34 E2E tests pass
✅ Linting passes (ruff, eslint)
Verification