Skip to content

fix: honor includeReferences parameter#1111

Open
3rabiii wants to merge 1 commit into
OneBusAway:mainfrom
3rabiii:fix-vehicles-for-agency-gap-3
Open

fix: honor includeReferences parameter#1111
3rabiii wants to merge 1 commit into
OneBusAway:mainfrom
3rabiii:fix-vehicles-for-agency-gap-3

Conversation

@3rabiii

@3rabiii 3rabiii commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes a spec gap where the vehicles-for-agency endpoint ignored the includeReferences parameter and always populated the references block.

Problem

The spec states that passing includeReferences=false should omit the references block to reduce payload size. The handler was unconditionally building and returning all references (agencies, routes, trips, situations), wasting both bandwidth and server resources.

Changes

  • Wrapped the references population logic in the existing ShouldIncludeReferences(r) helper.
  • When false, the response now correctly returns empty arrays for the references block using models.NewEmptyReferences().
  • Database queries for alerts/situations are now correctly bypassed when references are excluded, improving performance.
  • Added test coverage for includeReferences=false (asserting empty references) and true/absent (asserting populated references using table-driven tests).

Closes #1104

Summary by CodeRabbit

  • Bug Fixes
    • Vehicle list responses now respect the includeReferences option, returning reference data only when requested.
    • When reference data is disabled, the response still includes vehicles but leaves related reference sections empty.
    • Alerts and situation details are no longer fetched or added unless reference data is included.

Gate the population of the references block behind the
ShouldIncludeReferences helper. This correctly omits agencies,
routes, trips, stops, and situations when includeReferences=false,
reducing payload size and saving database alert queries.

Added unit tests to verify behavior for false, true, and absent states.
@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f9a30726-f577-4190-ba1a-54bc06bf5a5a

📥 Commits

Reviewing files that changed from the base of the PR and between c298c6e and e12e3a6.

📒 Files selected for processing (2)
  • internal/restapi/vehicles_for_agency_handler.go
  • internal/restapi/vehicles_for_agency_handler_test.go

📝 Walkthrough

Walkthrough

The vehicles-for-agency handler now conditionally populates references based on includeReferences. When false, the response keeps the vehicle list and returns empty reference collections. Tests cover false, absent, and true query values.

Changes

Vehicles-for-agency reference gating

Layer / File(s) Summary
Handler reference gating
internal/restapi/vehicles_for_agency_handler.go
Reference construction, alert deduplication, and situation references run only when ShouldIncludeReferences(r) is true; otherwise an empty references object is returned.
Query handling tests
internal/restapi/vehicles_for_agency_handler_test.go
Tests assert empty references for includeReferences=false and populated references for the absent and true cases.

Sequence Diagram(s)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • burma-shave
  • aaronbrethorst
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately states the main change.
Linked Issues check ✅ Passed The handler now honors includeReferences=false and returns empty references, with tests covering false, true, and absent cases.
Out of Scope Changes check ✅ Passed The changes stay focused on includeReferences handling and the associated tests, with no clear unrelated additions.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sonarqubecloud

Copy link
Copy Markdown

@3rabiii 3rabiii requested a review from burma-shave June 27, 2026 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[spec-gap] vehicles-for-agency: includeReferences request parameter is not honored

1 participant