Skip to content

feat: implement includeReferences query parameter in trip-details#1068

Open
3rabiii wants to merge 1 commit into
OneBusAway:mainfrom
3rabiii:fix-trip-details_issue_2
Open

feat: implement includeReferences query parameter in trip-details#1068
3rabiii wants to merge 1 commit into
OneBusAway:mainfrom
3rabiii:fix-trip-details_issue_2

Conversation

@3rabiii

@3rabiii 3rabiii commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR addresses by implementing the includeReferences query parameter for the trip-details endpoint.

Previously, the endpoint always built and serialized the full references object, ignoring caller intent. Now, when includeReferences=false is provided, the handler skips the reference-building logic entirely.

Changes Included:

  • Performance Optimization: Wrapped the reference-building block (agencies, trips, stops, routes, situations) in an if includeReferences condition. When false, we avoid unnecessary database queries, significantly speeding up the response.
  • Spec Compliance: When false, the references object is still present in the JSON envelope, but all its internal collections are initialized as empty arrays [], precisely matching the wiki specification.
  • Robust Testing:
    • Added TestTripDetailsHandlerWithIncludeReferencesFalse to assert all 5 reference collections are empty while entry data remains intact.
    • Added TestTripDetailsHandlerWithIncludeReferencesDefault (table-driven) to verify that both an absent parameter and an explicitly true parameter default to populating the references.

fixes: #1053

Summary by CodeRabbit

  • New Features
    • Trip Details API now includes an optional parameter to control reference data inclusion in responses. Disabling this feature reduces payload size by excluding reference details for agencies, routes, trips, stops, and situations. When omitted or enabled, references are populated as before, maintaining backward compatibility.

@coderabbitai

coderabbitai Bot commented Jun 13, 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: ee091e2a-aaab-44a5-8768-76814902858a

📥 Commits

Reviewing files that changed from the base of the PR and between 9898b8f and 11c22b5.

📒 Files selected for processing (2)
  • internal/restapi/trip_details_handler.go
  • internal/restapi/trip_details_handler_test.go

📝 Walkthrough

Walkthrough

The PR implements the includeReferences query parameter for the trip-details REST API endpoint. The handler now conditionally populates trip, agency, situation, stop, and route references based on the parameter value, allowing clients to reduce response size when they have cached copies of referenced entities. Two new integration tests verify the behavior when the parameter is false and when it defaults to true.

Changes

Include References Parameter Implementation

Layer / File(s) Summary
Conditional references population in handler
internal/restapi/trip_details_handler.go
Handler now gates trip, agency, situation, stop, and route reference building behind ShouldIncludeReferences(r). When disabled, the references object remains in the response with empty collections; when enabled, references are built and appended as before.
Test coverage for includeReferences parameter
internal/restapi/trip_details_handler_test.go
Two new integration tests verify that Data.References collections are empty when includeReferences=false and properly populated for default/true cases, while Data.Entry remains populated in both scenarios.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

  • OneBusAway/maglev#1047: This PR directly fixes the issue where the trip-details handler ignored the includeReferences query parameter by gating all reference population behind ShouldIncludeReferences.

Possibly related PRs

  • OneBusAway/maglev#1019: Both PRs implement the includeReferences query parameter by conditionally gating whether references sublists are populated in REST handlers.
  • OneBusAway/maglev#913: Both PRs update trip_details_handler_test.go to assert Data.References contents in response to inclusion flags.

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 accurately describes the primary change: implementing the includeReferences query parameter in the trip-details endpoint.
Linked Issues check ✅ Passed The PR implementation meets all coding requirements from issue #1053: conditionally populates references based on includeReferences parameter, returns empty arrays when false, and includes test coverage.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the includeReferences parameter as specified in issue #1053; no out-of-scope modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 and usage tips.

@sonarqubecloud

Copy link
Copy Markdown

@3rabiii 3rabiii requested a review from burma-shave June 14, 2026 22:30
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.

trip-details: implement includeReferences query parameter

1 participant