Ignore soft-404 pages when scraping product prices - #201
Conversation
Many storefronts return HTML 404s with a page title and promotional banner amounts. Naive price selectors then store those as the product price. Clear price and mark Discontinued when common soft-404 markers are present.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesSoft 404 scraping guards
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/Unit/Services/ScrapeUrlTest.php (1)
280-290: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover canonical
/404detection independently.Line 280 omits the canonical branch; the integration fixture already matches via its title and
template-404class. Add direct cases for bothrel/hrefattribute orders so a canonical-regex regression is caught.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Unit/Services/ScrapeUrlTest.php` around lines 280 - 290, Add direct assertions to test_looks_like_not_found_page_detects_common_markers for canonical /404 links, covering both rel-before-href and href-before-rel attribute orders. Keep the existing marker and non-404 assertions unchanged so canonical detection is tested independently of title or template-404 matching.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/Unit/Services/ScrapeUrlTest.php`:
- Around line 280-290: Add direct assertions to
test_looks_like_not_found_page_detects_common_markers for canonical /404 links,
covering both rel-before-href and href-before-rel attribute orders. Keep the
existing marker and non-404 assertions unchanged so canonical detection is
tested independently of title or template-404 matching.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 202d840a-f6c3-4956-b34b-452147e2a5b0
📒 Files selected for processing (2)
app/Services/ScrapeUrl.phptests/Unit/Services/ScrapeUrlTest.php
Assert both rel/href attribute orders for canonical /404 links so that branch is tested independently of title and template-404 markers.
|
Addressed CodeRabbit nit: added direct canonical |
|
thanks, merged here #214 as it needed a bunch of fixes |
Summary
template-404, canonical/404, etc.)https://schema.org/DiscontinuedWhy
Soft 404 HTML still has a title and often a promotional banner with a dollar amount (e.g. “Orders Over $899!”). Naive price selectors then store that banner amount as the product price, which shows up as a false “deal.”
Related discussion: out-of-stock / unavailable URL handling (#166).
Test plan
Made with Cursor
Summary by CodeRabbit
Bug Fixes
https://schema.org/Discontinuedto prevent misleading availability/threshold-based pricing.Tests