Skip to content

fix: honor time request param in vehicles-for-agency API#1106

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

fix: honor time request param in vehicles-for-agency API#1106
3rabiii wants to merge 2 commits into
OneBusAway:mainfrom
3rabiii:fix-vehicles-for-agency-gap-1

Conversation

@3rabiii

@3rabiii 3rabiii commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes a spec gap in the vehicles-for-agency endpoint where the time request parameter was previously ignored. The handler now properly parses the supplied time to use as the reference time, falling back to the server clock if absent, as required by the OpenAPI spec.

Problem

According to the specification, the time parameter should set the reference time used for trip resolution. However, the handler was unconditionally using api.Clock.Now(). This meant that historical or future time parameters sent by clients were silently ignored.

Changes

  • Parse the time query parameter using utils.ParseTimeParameter.
  • Fall back to api.Clock.Now() if the time parameter is absent.
  • Return an HTTP 400 validation error if the provided time is unparseable.
  • Use the parsed reference time instead of the server clock to populate currentTime for entry/trip-status timestamps.
  • Add three new tests to verify behavior with epoch-ms input, absent input, and invalid string input.

Closes #1102

Summary by CodeRabbit

  • Bug Fixes
    • /vehicles-for-agency now uses a consistent reference time when generating vehicle and trip timestamps.
    • Added support for an optional time query parameter so responses can reflect a specific moment in time.
    • Invalid time values now return a clear validation error instead of producing a response.
    • When time is not provided, the endpoint continues to use the current server time.

Parse the 'time' query parameter in the vehicles-for-agency endpoint
to set the reference time, falling back to the server clock if absent.
This fixes a spec gap where the parameter was silently ignored.

Add validation to return HTTP 400 on invalid time formats using the
existing ParseTimeParameter utility. Add test coverage for epoch-ms,
absent, and invalid inputs.
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@3rabiii, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 48 minutes and 7 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4738ab67-a1ae-4748-8f48-af287ecea116

📥 Commits

Reviewing files that changed from the base of the PR and between 80b3336 and 2070efd.

📒 Files selected for processing (1)
  • internal/restapi/vehicles_for_agency_handler.go
📝 Walkthrough

Walkthrough

The vehicles-for-agency handler now uses an optional time query parameter to set the reference time for vehicle and trip status timestamps, falls back to the server clock when it is absent, and returns a validation error for invalid values. Tests cover epoch-millisecond, missing, and invalid input cases.

Changes

Vehicles-for-agency reference time

Layer / File(s) Summary
Reference time resolution and validation
internal/restapi/vehicles_for_agency_handler.go, internal/restapi/vehicles_for_agency_handler_test.go
vehiclesForAgencyHandler parses the request time parameter, uses the server clock when it is absent, applies the resulting reference time to vehicle and trip status timestamps, and the tests cover valid epoch-millisecond, missing, and invalid inputs.

Estimated review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • burma-shave
  • fletcherw
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Concise and specific; it clearly states the handler now honors the time parameter.
Linked Issues check ✅ Passed The handler now parses time, defaults to server time when absent, rejects invalid input, and uses the derived reference time for vehicle/trip timing.
Out of Scope Changes check ✅ Passed The changes stay focused on the vehicles-for-agency time parameter and matching tests; no unrelated behavior or files were introduced.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

✏️ 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@internal/restapi/vehicles_for_agency_handler.go`:
- Around line 32-47: The request handler currently calls loadAgencyLocation
unconditionally in vehiclesForAgencyHandler, which introduces a new failure path
for requests that do not include the time query param. Move the
loadAgencyLocation(agency.ID, agency.Timezone) call inside the timeParam branch
in the handler so it is only executed when ParseTimeParameter is needed, and
keep the existing api.validationErrorResponse and api.serverErrorResponse
handling around that branch.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 547f3b29-c869-4f07-b6c2-5bc288e01f32

📥 Commits

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

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

Comment thread internal/restapi/vehicles_for_agency_handler.go
Loading the agency timezone unconditionally risks a 500 error even
when the time parameter is absent. Moving this call inside the if
block ensures it only runs when needed, improving performance and
preventing regressions.

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@sonarqubecloud

Copy link
Copy Markdown

@3rabiii 3rabiii requested a review from burma-shave June 25, 2026 15:54
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: time request parameter is ignored

1 participant