Repository navigation
feat: shopping list with link previews and price drop alerts - #274
Open
TheBlackSide243 wants to merge 2 commits into
Open
TheBlackSide243 wants to merge 2 commits into
TheBlackSide243 wants to merge 2 commits into
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reviewer's GuideThis PR introduces an end-to-end shopping-list feature: authenticated wishlist storage and management, SSRF-safe product-link previews, wardrobe-gap suggestions, daily and manual price monitoring with notifications, and a localized dashboard UI backed by migrations, workers, hooks, and focused tests. Sequence diagram for wishlist link preview and price checkingsequenceDiagram
actor User
participant UI as WishlistPage
participant API as WishlistAPI
participant Preview as link_preview
participant Shop as ShopPage
participant DB as wishlist_items
participant Worker as PriceWatchWorker
participant Notify as NotificationDispatcher
User->>UI: Paste shop link
UI->>API: preview_link
API->>Preview: fetch_preview
Preview->>Shop: GET product page
Shop-->>Preview: HTML metadata and product data
Preview-->>API: LinkPreview
API-->>UI: title, image_url, price
UI->>API: add_wishlist_item
API->>DB: Store wishlist item
User->>UI: Check prices
UI->>API: check_prices_now
API->>Preview: check_prices
Preview->>Shop: fetch_preview
Shop-->>Preview: Current price
Preview-->>API: apply_price
API-->>UI: Updated wishlist entries
Worker->>DB: due_query
Worker->>Preview: check_prices
Preview->>Shop: fetch_preview
Shop-->>Preview: Current price
Preview-->>Worker: PriceDrop
Worker->>Notify: deliver
Notify-->>User: Price drop notification
Entity relationship diagram for wishlist price trackingerDiagram
USERS ||--o{ WISHLIST_ITEMS : owns
USERS {
UUID id PK
}
WISHLIST_ITEMS {
UUID id PK
UUID user_id FK
string name
string link
decimal price
decimal current_price
datetime price_checked_at
datetime price_dropped_at
boolean bought
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
github-actions
Bot
added
javascript
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 7 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="backend/app/services/link_preview.py" line_range="193" />
<code_context>
+ headers={"User-Agent": USER_AGENT, "Accept": "text/html,application/xhtml+xml"},
+ ) as client:
+ for _ in range(MAX_REDIRECTS + 1):
+ await ensure_public_host(urlsplit(current).hostname or "")
+ try:
+ async with client.stream("GET", current) as response:
</code_context>
<issue_to_address>
**Private services become reachable**
When a hostname resolves to a public address during validation and a private address when HTTPX connects, `ensure_public_host` checks one DNS answer, but `client.stream` resolves the hostname again; the preview request can therefore reach an internal service.
Pin the validated address for the connection, or otherwise ensure the connected address is the one that passed the public-host check.
</issue_to_address>
### Comment 2
<location path="backend/app/services/link_preview.py" line_range="193" />
<code_context>
+ headers={"User-Agent": USER_AGENT, "Accept": "text/html,application/xhtml+xml"},
+ ) as client:
+ for _ in range(MAX_REDIRECTS + 1):
+ await ensure_public_host(urlsplit(current).hostname or "")
+ try:
+ async with client.stream("GET", current) as response:
</code_context>
<issue_to_address>
**DNS preflight has no timeout**
When the system DNS resolver is slow or unresponsive, `ensure_public_host` awaits the system resolver without a deadline, and the HTTPX timeout does not cover this earlier lookup. A stalled DNS resolver therefore keeps a preview request waiting indefinitely and can hold a price-check worker job and its distributed lock until the outer worker timeout intervenes.
Put a bounded timeout around the DNS preflight and handle expiry as a `PreviewError`.
</issue_to_address>
### Comment 3
<location path="backend/app/workers/price_watch.py" line_range="43-45" />
<code_context>
+ if user is None:
+ continue
+ try:
+ await dispatcher.deliver(user_id, drop_message(user, user_drops))
+ except Exception as e:
+ logger.warning(f"Price drop notification failed for {user_id}: {e}")
+ logger.info(f"Wishlist prices: checked {len(entries)}, {len(drops)} drops")
+ return {"checked": len(entries), "drops": len(drops)}
</code_context>
<issue_to_address>
**Price-drop alerts disappear**
When a drop is detected and all channels fail, no channels are enabled, or delivery raises, `dispatcher.deliver` results are discarded and raised exceptions are only logged. Because the price state is committed first, later checks do not report the same drop again, so the user never receives the alert.
Record delivery attempts and schedule failed or undeliverable alerts for retry.
</issue_to_address>
### Comment 4
<location path="backend/app/services/price_watch.py" line_range="66" />
<code_context>
+ ]
+ if user_id is not None:
+ conditions.append(WishlistItem.user_id == user_id)
+ return select(WishlistItem).where(and_(*conditions)).order_by(WishlistItem.price_checked_at)
+
+
</code_context>
<issue_to_address>
**New links remain unchecked**
When more than 200 eligible items remain due across daily runs, `due_query` orders by `price_checked_at` ascending, which places unchecked NULL timestamps after dated rows in PostgreSQL. Previously checked rows become due again and stay ahead of new links, so the worker repeatedly selects the same batch and leaves later links unchecked.
Order unchecked rows before dated rows, or otherwise ensure the limited batch advances through all eligible items.
Also at `backend/app/workers/price_watch.py:13`, `backend/app/workers/price_watch.py:28`.
</issue_to_address>
### Comment 5
<location path="backend/app/services/price_watch.py" line_range="44-48" />
<code_context>
+ if entry.price is None:
+ entry.price = price
+ return None
+ dropped = entry.price - price >= MIN_DROP
+ new_low = previous is None or price < previous
+ if dropped and new_low:
+ entry.price_dropped_at = now
+ return PriceDrop(entry, entry.price, price)
+ if not dropped:
+ entry.price_dropped_at = None
</code_context>
<issue_to_address>
**Alerts use stale reference prices**
When the user changes an item's saved price while a scheduled check is fetching its shop page, `apply_price` compares against the previously loaded `entry.price` and uses it in `PriceDrop`, so the worker can report a drop that is no longer warranted and persist monitoring state based on the stale reference.
Reload or lock the item before applying the fetched price, and compare against its current saved reference price.
Also at `backend/app/api/wishlist.py:188`.
</issue_to_address>
### Comment 6
<location path="backend/app/api/wishlist.py" line_range="188" />
<code_context>
+ for key, value in data.model_dump(exclude_unset=True).items():
+ if key in ("name", "priority", "bought") and value is None:
+ continue
+ setattr(entry, key, value)
+ await db.flush()
+ await db.refresh(entry)
</code_context>
<issue_to_address>
**Edited links retain old price state**
When a caller patches an item's link or reference price, `update_wishlist_item` changes the link or price without clearing `current_price`, `price_checked_at`, or `price_dropped_at`. The item can keep showing an outdated drop, compare a new shop against the old shop's price, or wait for the old check interval.
Reset the monitoring state when the item's link or reference price changes.
</issue_to_address>
### Comment 7
<location path="backend/app/services/price_watch.py" line_range="94-100" />
<code_context>
+
+
+def drop_message(user: User, drops: list[PriceDrop]) -> NotificationMessage:
+ italian = (user.locale or "").startswith("it")
+ lines = [f"{d.entry.name}: {_money(d.old)} → {_money(d.new)}" for d in drops[:5]]
+ if italian:
+ title = "Prezzo sceso" if len(drops) == 1 else f"{len(drops)} prezzi scesi"
+ label = "Apri Cosa comprare"
+ else:
+ title = "Price drop" if len(drops) == 1 else f"{len(drops)} price drops"
+ label = "Open shopping list"
+ return NotificationMessage(
</code_context>
<issue_to_address>
**Alerts ignore most locales**
When a user has a supported locale other than Italian, `drop_message` translates alerts only for Italian users and sends English text to everyone else. Users in the other supported locales receive price-drop notifications in English despite the localized shopping-list feature.
Provide translated alert titles, labels, and bodies for all supported locales, with a deliberate fallback for unknown locales.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 7 findings to address first, and the server checks that a URL resolves to public IPs, then httpx resolves it again when connecting; a hostname that changes its DNS answer between those steps could make the server fetch a private service. Reverting stops future fetches, but cannot undo any internal data exposed, and this public-only check is the trust boundary for every preview and price check.
Blocking findings: backend/app/services/link_preview.py:193, backend/app/services/link_preview.py:193, backend/app/workers/price_watch.py:45, backend/app/services/price_watch.py:66, backend/app/services/price_watch.py:48, and 2 more
| headers={"User-Agent": USER_AGENT, "Accept": "text/html,application/xhtml+xml"}, | ||
| ) as client: | ||
| for _ in range(MAX_REDIRECTS + 1): | ||
| await ensure_public_host(urlsplit(current).hostname or "") |
There was a problem hiding this comment.
🔴 Critical · Private services become reachable
When a hostname resolves to a public address during validation and a private address when HTTPX connects, ensure_public_host checks one DNS answer, but client.stream resolves the hostname again; the preview request can therefore reach an internal service.
Pin the validated address for the connection, or otherwise ensure the connected address is the one that passed the public-host check.
Prompt for AI agents
In `backend/app/services/link_preview.py` at line 193:
**Private services become reachable**
When a hostname resolves to a public address during validation and a private address when HTTPX connects, `ensure_public_host` checks one DNS answer, but `client.stream` resolves the hostname again; the preview request can therefore reach an internal service.
Pin the validated address for the connection, or otherwise ensure the connected address is the one that passed the public-host check.| headers={"User-Agent": USER_AGENT, "Accept": "text/html,application/xhtml+xml"}, | ||
| ) as client: | ||
| for _ in range(MAX_REDIRECTS + 1): | ||
| await ensure_public_host(urlsplit(current).hostname or "") |
There was a problem hiding this comment.
🟡 Medium · DNS preflight has no timeout
When the system DNS resolver is slow or unresponsive, ensure_public_host awaits the system resolver without a deadline, and the HTTPX timeout does not cover this earlier lookup. A stalled DNS resolver therefore keeps a preview request waiting indefinitely and can hold a price-check worker job and its distributed lock until the outer worker timeout intervenes.
Put a bounded timeout around the DNS preflight and handle expiry as a PreviewError.
Prompt for AI agents
In `backend/app/services/link_preview.py` at line 193:
**DNS preflight has no timeout**
When the system DNS resolver is slow or unresponsive, `ensure_public_host` awaits the system resolver without a deadline, and the HTTPX timeout does not cover this earlier lookup. A stalled DNS resolver therefore keeps a preview request waiting indefinitely and can hold a price-check worker job and its distributed lock until the outer worker timeout intervenes.
Put a bounded timeout around the DNS preflight and handle expiry as a `PreviewError`.
Comment on lines
+43
to
+45
| await dispatcher.deliver(user_id, drop_message(user, user_drops)) | ||
| except Exception as e: | ||
| logger.warning(f"Price drop notification failed for {user_id}: {e}") |
There was a problem hiding this comment.
🟡 Medium · Price-drop alerts disappear
When a drop is detected and all channels fail, no channels are enabled, or delivery raises, dispatcher.deliver results are discarded and raised exceptions are only logged. Because the price state is committed first, later checks do not report the same drop again, so the user never receives the alert.
Record delivery attempts and schedule failed or undeliverable alerts for retry.
Prompt for AI agents
In `backend/app/workers/price_watch.py` at lines 43-45:
**Price-drop alerts disappear**
When a drop is detected and all channels fail, no channels are enabled, or delivery raises, `dispatcher.deliver` results are discarded and raised exceptions are only logged. Because the price state is committed first, later checks do not report the same drop again, so the user never receives the alert.
Record delivery attempts and schedule failed or undeliverable alerts for retry.| ] | ||
| if user_id is not None: | ||
| conditions.append(WishlistItem.user_id == user_id) | ||
| return select(WishlistItem).where(and_(*conditions)).order_by(WishlistItem.price_checked_at) |
There was a problem hiding this comment.
🟡 Medium · New links remain unchecked
When more than 200 eligible items remain due across daily runs, due_query orders by price_checked_at ascending, which places unchecked NULL timestamps after dated rows in PostgreSQL. Previously checked rows become due again and stay ahead of new links, so the worker repeatedly selects the same batch and leaves later links unchecked.
Order unchecked rows before dated rows, or otherwise ensure the limited batch advances through all eligible items.
Also at backend/app/workers/price_watch.py:13, backend/app/workers/price_watch.py:28.
Prompt for AI agents
In `backend/app/services/price_watch.py` at line 66:
**New links remain unchecked**
When more than 200 eligible items remain due across daily runs, `due_query` orders by `price_checked_at` ascending, which places unchecked NULL timestamps after dated rows in PostgreSQL. Previously checked rows become due again and stay ahead of new links, so the worker repeatedly selects the same batch and leaves later links unchecked.
Order unchecked rows before dated rows, or otherwise ensure the limited batch advances through all eligible items.
Also at `backend/app/workers/price_watch.py:13`, `backend/app/workers/price_watch.py:28`.
Comment on lines
+44
to
+48
| dropped = entry.price - price >= MIN_DROP | ||
| new_low = previous is None or price < previous | ||
| if dropped and new_low: | ||
| entry.price_dropped_at = now | ||
| return PriceDrop(entry, entry.price, price) |
There was a problem hiding this comment.
🟡 Medium · Alerts use stale reference prices
When the user changes an item's saved price while a scheduled check is fetching its shop page, apply_price compares against the previously loaded entry.price and uses it in PriceDrop, so the worker can report a drop that is no longer warranted and persist monitoring state based on the stale reference.
Reload or lock the item before applying the fetched price, and compare against its current saved reference price.
Also at backend/app/api/wishlist.py:188.
Prompt for AI agents
In `backend/app/services/price_watch.py` at lines 44-48:
**Alerts use stale reference prices**
When the user changes an item's saved price while a scheduled check is fetching its shop page, `apply_price` compares against the previously loaded `entry.price` and uses it in `PriceDrop`, so the worker can report a drop that is no longer warranted and persist monitoring state based on the stale reference.
Reload or lock the item before applying the fetched price, and compare against its current saved reference price.
Also at `backend/app/api/wishlist.py:188`.| for key, value in data.model_dump(exclude_unset=True).items(): | ||
| if key in ("name", "priority", "bought") and value is None: | ||
| continue | ||
| setattr(entry, key, value) |
There was a problem hiding this comment.
🟡 Medium · Edited links retain old price state
When a caller patches an item's link or reference price, update_wishlist_item changes the link or price without clearing current_price, price_checked_at, or price_dropped_at. The item can keep showing an outdated drop, compare a new shop against the old shop's price, or wait for the old check interval.
Reset the monitoring state when the item's link or reference price changes.
Prompt for AI agents
In `backend/app/api/wishlist.py` at line 188:
**Edited links retain old price state**
When a caller patches an item's link or reference price, `update_wishlist_item` changes the link or price without clearing `current_price`, `price_checked_at`, or `price_dropped_at`. The item can keep showing an outdated drop, compare a new shop against the old shop's price, or wait for the old check interval.
Reset the monitoring state when the item's link or reference price changes.
Comment on lines
+94
to
+100
| italian = (user.locale or "").startswith("it") | ||
| lines = [f"{d.entry.name}: {_money(d.old)} → {_money(d.new)}" for d in drops[:5]] | ||
| if italian: | ||
| title = "Prezzo sceso" if len(drops) == 1 else f"{len(drops)} prezzi scesi" | ||
| label = "Apri Cosa comprare" | ||
| else: | ||
| title = "Price drop" if len(drops) == 1 else f"{len(drops)} price drops" |
There was a problem hiding this comment.
🟡 Medium · Alerts ignore most locales
When a user has a supported locale other than Italian, drop_message translates alerts only for Italian users and sends English text to everyone else. Users in the other supported locales receive price-drop notifications in English despite the localized shopping-list feature.
Provide translated alert titles, labels, and bodies for all supported locales, with a deliberate fallback for unknown locales.
Prompt for AI agents
In `backend/app/services/price_watch.py` at lines 94-100:
**Alerts ignore most locales**
When a user has a supported locale other than Italian, `drop_message` translates alerts only for Italian users and sends English text to everyone else. Users in the other supported locales receive price-drop notifications in English despite the localized shopping-list feature.
Provide translated alert titles, labels, and bodies for all supported locales, with a deliberate fallback for unknown locales.- Link previews connect to the address that passed the public-host check (Host header and TLS server name keep the shop's name), so a second DNS answer cannot point the request into the LAN; the lookup has a timeout. - Never-checked links are read before old ones (NULLS FIRST). - The reference price is re-read after the shop page loads; a changed link or a bought entry drops the result. - Editing the link or the price starts the watch over. - A drop whose alert could not be delivered is reported again next time. - Alerts are written in all eight supported languages. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a Shopping list page for clothes the user wants to buy: a name, notes, a priority, a type,
a price, and either a photo or a shop link. Pasting a link fills in the title, picture and price
from the page. The list also suggests what the wardrobe is short of, by comparing the number of
items per role with a small target (e.g. only one pair of shoes).
Saved links are re-checked once a day (arq cron, 09:05 worker time) and the user gets a notification through
the existing notification channels when a price goes down; a "Check prices" button does it on
demand.
wishlist_itemstable (migrationsd7e1f3a9c2b4,e8f2a4b6c1d9), CRUD under/api/v1/wishlist,POST /wishlist/preview,GET /wishlist/suggestions,POST /wishlist/check-prices.redirect, including IPv4-mapped IPv6), manual redirects, size and time limits.
/dashboard/wishlist, nav entry, translations for all 8 locales.This is one of a set of small, independent PRs (each branches from
mainand can be reviewed ordropped on its own); they only overlap in the nav/router registration lines.
Note on migrations
This PR and the declutter one each add a migration on top of
a7d3c9e1f5b2. They areindependent, so whichever is merged second needs its first migration's
down_revisionbumped tothe other's head; happy to rebase it.
Type of Change
Checklist
Testing
ruff check,ruff format --check, migration heads check and the full pytest suite on top of currentmain(2045 passed).npm run lint,i18n:check,vocab:check,tsc --noEmitand vitest (693 passed).Test Environment
🤖 Generated with Claude Code
Summary by Sourcery
Add an end-to-end shopping list with wardrobe suggestions, safe shop-link previews, and automated price-drop alerts.
New Features:
Enhancements:
Tests:
Chores: