Skip to content

fix: trip endpoint spec gaps — routeShortName source and includeReferences#1063

Open
Ahmedhossamdev wants to merge 1 commit into
mainfrom
fix/trip-endpoint-spec-gaps
Open

fix: trip endpoint spec gaps — routeShortName source and includeReferences#1063
Ahmedhossamdev wants to merge 1 commit into
mainfrom
fix/trip-endpoint-spec-gaps

Conversation

@Ahmedhossamdev

@Ahmedhossamdev Ahmedhossamdev commented Jun 12, 2026

Copy link
Copy Markdown
Member

Fixes: #987, #1062

Fixes two spec gaps in /api/where/trip/{id}, both verified byte-for-byte against the Java reference server.

1. entry.routeShortName sourced from the wrong table
It read the route's short name; the spec/Java define it as the trip's own per-trip route short name (GTFS trips.txt route_short_name). Maglev doesn't store that column, so the parity-correct value is "". Clients fall back to references.routes[].shortName as the spec instructs.

2. includeReferences=false ignored
The handler never read the parameter and always populated references. Now parses it (default true) and returns empty reference arrays when false.

Summary by CodeRabbit

  • New Features

    • Added includeReferences query parameter to the trip endpoint. When set to false, reference data (routes, agencies, stops, trips, situations) are excluded from responses. References are included by default.
  • Bug Fixes

    • Updated route short name field to reflect correct stored values.
  • Tests

    • Added test coverage for the new includeReferences parameter functionality.

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The trip endpoint handler is updated to remove per-trip route short name population (hardcoded to empty string) and to support conditional reference inclusion via an includeReferences query parameter. When the parameter is false, reference arrays remain empty; when true or absent (default), routes and agencies are populated.

Changes

Trip endpoint response handling

Layer / File(s) Summary
RouteShortName field removal
internal/restapi/trip_handler.go, internal/restapi/trip_handler_test.go
Trip response RouteShortName is hardcoded to empty string instead of sourcing from referenced route; test assertion updated to expect empty string.
IncludeReferences query parameter support
internal/restapi/trip_handler.go, internal/restapi/trip_handler_test.go
Handler parses includeReferences query parameter (defaults to true) and conditionally populates routes and agencies references; new test validates all three states: parameter absent, false, and true.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

Possibly related PRs

  • OneBusAway/maglev#1019: Implements the same includeReferences query-parameter pattern in route handler via ShouldIncludeReferences helper.
  • OneBusAway/maglev#957: Previously updated TestTripHandlerEndToEnd assertions on trip entry response structure.

Suggested reviewers

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

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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
Title check ✅ Passed The title clearly and concisely summarizes the two main changes: fixing routeShortName source and adding includeReferences parameter support.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/trip-endpoint-spec-gaps

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

@github-actions

Copy link
Copy Markdown

Performance Smoke Test Results

Status: PASSED

Metric Value
p(95) latency 2.0 ms
Error rate 0.00%
Total requests 344
Req/sec 11.3

Smoke test config: 5 VUs x 30s. Thresholds: p(95) < 300ms, error rate < 1%.

Full results uploaded as workflow artifact: k6-smoke-summary.

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 review: trip

1 participant