Skip to content

Conversation

@MohamedKamal000
Copy link
Contributor

@MohamedKamal000 MohamedKamal000 commented Dec 30, 2025

solves #108
Verification & Testing Checklist for trip-details Endpoint:

i tested the endpoint against pugetsound server with agency id 1 and 40

[x] Verify the endpoint is reachable and responding with correct status codes (e.g., 200 OK for valid requests).

[x] Compare the response from testing against production to ensure consistency in data and structure.

[x] Validate response schema (fields, types, required properties).
currently there is only one type change against the server
in the trips field inside the references field, the directionId is integer in the development
but it is string in the server response
So I iam not sure should i change or not. i didn't want to because it would result in more refactoring across the codebase

[x] Confirm trip details are accurate for the given trip ID.

[x] Test with different trip IDs (valid, invalid, non-existent).

[x] Test edge cases (e.g., trips with missing fields).

[] Check response times and ensure performance is acceptable. (the direction calculation causes request time out, see issue #165 )

[x] Ensure proper error handling (400, 404, 500 cases).

[] Write/strengthen unit tests if coverage is missing or weak.

[x] Confirm no regressions compared to production behavior.

Notes:

  • in order for me to test the end point correclty, i had to use the DirectionCalculator in the
    precomputation step, so that i don't wait for a long times and maybe don't even get a response when testing
    when i did that, i noticed difference in the direction values compared to the server response

  • i didn't test the status field much, since it depends on real time data

Copy link
Member

@aaronbrethorst aaronbrethorst left a comment

Choose a reason for hiding this comment

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

Thanks for putting together this verification PR for the trip-details endpoint! The testing checklist in the PR description is thorough, and it's clear you've done substantial comparison against the production Puget Sound server. The changes here address real discrepancies between Maglev and the production API.


Issues to Fix

1. Inconsistent Service Date Fix

The service date calculation fix in trip_details_handler.go is correct—using time.Date() with the proper timezone is the right approach since Truncate(24 * time.Hour) truncates to UTC midnight, not local midnight:

// Fixed in this PR (correct):
y, m, d := currentTime.Date()
serviceDate = time.Date(y, m, d, 0, 0, 0, 0, loc)

However, the same problematic pattern exists in trip_for_vehicle_handler.go:103:

serviceDate = currentTime.Truncate(24 * time.Hour)  // Still uses the old approach

What needs to change:

  • Apply the same fix to trip_for_vehicle_handler.go for consistency
  • This handler shares similar logic and parameters with trip_details_handler.go

Suggestions

Consider centralizing wheelchair boarding constants usage

The new constants (Accessible, NotAccessible) are well-placed in constants.go. You might want to check if there are other places in the codebase using hardcoded "ACCESSIBLE" or "NOT_ACCESSIBLE" strings that should be updated for consistency:

grep -r '"ACCESSIBLE"\|"NOT_ACCESSIBLE"' internal/

The directionId type discrepancy

You noted in the PR description:

in the trips field inside the references field, the directionId is integer in the development but it is string in the server response

This is a valid observation. The GTFS spec defines direction_id as an enum (0 or 1), so integer is arguably more correct, but matching production behavior is important for compatibility. This might warrant a separate follow-up issue.


What Looks Good

  • Vehicle ID fix in trips_helper.go:33 correctly uses FormCombinedID to match the expected format
  • Wheelchair boarding mapping now properly uses the utility function rather than hardcoding "UNKNOWN"
  • JSON tag changes in trip_details.go (removing omitempty from frequency and situationIds) align with production behavior
  • The constants approach in models/constants.go improves maintainability

Once the service date fix is applied consistently to trip_for_vehicle_handler.go, this PR is good to go!

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.

2 participants