fix(stops-for-location): send the longitude span as lonSpan - #612
fix(stops-for-location): send the longitude span as lonSpan#612omlahore wants to merge 1 commit into
Conversation
MapView requests stops for the visible bounding box with lngSpan, and the endpoint forwarded that name straight to the SDK. The SDK parameter is lonSpan, so the longitude span was dropped and the query fell back to the default window.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe stops-for-location OBA route now sends the parsed ChangesOBA stops-for-location query
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to Stops-for-location requests now preserve the map’s longitude span when calling OBA, preventing edge stops from being omitted in wide map views. The corrected parameter mapping is covered by a focused regression test. 🚥 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 |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
What this fixes
/api/oba/stops-for-locationforwards the client'slngSpanstraight through to the SDK (+server.js#L8-L18):The SDK's parameter is
lonSpan, declared inonebusaway-sdk/resources/stops-for-location.d.ts:lngSpanis not a parameter it knows, so the longitude span was dropped and the request fell back to the default window.How it shows up
MapViewsends a real span computed from the visible bounding box (MapView.svelte#L192):`/api/oba/stops-for-location?lat=${lat}&lng=${lng}&latSpan=${boundingBox.north - boundingBox.south}&lngSpan=${boundingBox.east - boundingBox.west}&radius=1500`So on a wide viewport the east-west extent the user can see is wider than the window actually queried, and stops near the left and right edges are missing from the map.
The rest of the codebase already uses
lonSpan(serverCache.test.js,mathUtils.test.js), which is what makes this one the outlier.Change
One line, keeping the existing
lngtolonmapping style in the same object:The client keeps sending
lngSpan; only the name handed to the SDK changes.Test
New
src/tests/api/stops-for-location.test.jsasserts the SDK receiveslonSpan. On unmodified develop:Passes with the change. Prettier clean.
Unrelated note:
src/tests/lib/dateTimeFormat.test.jsfails locally for me on develop too, expecting9:15 AMwhere anen-INlocale renders9:15 am. Not touched here.Summary by CodeRabbit
Bug Fixes
Tests