Fix #35: Prevent duplicate products in Fastest Delivery sort #98
Conversation
When sorting by 'Fastest Delivery', products with multiple delivery options were appearing as duplicates due to SQL JOIN with ProductDeliveryLink table creating multiple rows. Added .distinct() to all delivery_fastest sorting queries to ensure each product appears exactly once in results. - Fixed duplicate items when no delivery filter selected - Fixed duplicate items when delivery option filter applied - All 51 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 diff adds .distinct() calls to multiple query branches in the product API endpoint to eliminate duplicate results.
Key concerns:
- The diff shows
.distinct()being added to only some conditional branches. Without seeing the complete file, I cannot confirm if all branches are consistently handled, but the visibleprice_ascbranch at line 261 lacks the.distinct()call while others have it. - The
.distinct()placement after.order_by()is unusual and may cause database-specific issues. Standard practice is to apply DISTINCT before ordering.
These inconsistencies could lead to different behavior depending on which sorting option the user selects, which is likely unintended.
| ).asc(), | ||
| cast(ColumnElement[float], Product.price).asc(), | ||
| ) | ||
| .distinct() |
There was a problem hiding this comment.
The .distinct() call is placed after .order_by(). In SQLAlchemy (and SQL in general), DISTINCT operations should typically be applied before ordering to ensure correct behavior across different database backends. This order may cause issues with PostgreSQL's DISTINCT ON or when the ORDER BY columns aren't in the SELECT list. Consider moving .distinct() before the .order_by() call.
| .distinct() | ||
| ) | ||
| elif sort == "price_asc": | ||
| stmt = stmt.order_by(cast(ColumnElement[float], Product.price).asc()) |
There was a problem hiding this comment.
Inconsistent application of .distinct(): The price_asc sorting branch doesn't include .distinct() while other sorting branches do. This inconsistency suggests either a missing .distinct() call here (if deduplication is required for all queries) or unnecessary .distinct() calls in other branches (if deduplication isn't needed). This should be made consistent across all branches.
Fixes #35
Problem
Duplicate items were showing when the Fastest Delivery sort option was selected. This occurred because the SQL query joins with the ProductDeliveryLink table, creating duplicate rows when a product has multiple delivery options.
Solution
Added
.distinct()to alldelivery_fastestsorting queries to ensure each product appears exactly once in the results, regardless of how many delivery options it has.Testing
✅ All 51 backend tests pass
✅ All 34 E2E tests pass
✅ All CI checks pass
✅ Tested with and without category filters
✅ Tested with and without delivery option filters
Changes
Modified
/api/productsendpoint inbackend/app/main.py:.distinct()when delivery filter is applied with express speed.distinct()when delivery filter is applied with other speeds.distinct()when no delivery filter is appliedThis ensures no duplicate products appear in any Fastest Delivery sorting scenario.