Skip to content

fix: omit schedule field from trip-details response when nil#1073

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

fix: omit schedule field from trip-details response when nil#1073
3rabiii wants to merge 1 commit into
OneBusAway:mainfrom
3rabiii:fix-trip-details_issue_6

Conversation

@3rabiii

@3rabiii 3rabiii commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR addresses to ensure strict compliance with the wiki spec regarding the schedule field in the trip-details endpoint.

The spec explicitly states: "The schedule key is absent from the entry entirely" when includeSchedule=false. Previously, the field lacked the omitempty tag, causing the JSON encoder to emit "schedule": null when the schedule pointer was nil.

Changes Included:

  • JSON Tag Update: Added the omitempty tag to the Schedule field within the TripDetails model (internal/models/trip_details.go). This ensures the key is completely dropped from the JSON payload when the schedule is not requested, matching the expected contract behavior.

Fixes: #1058

Summary by CodeRabbit

  • Refactor
    • Optimized trip details data format to exclude empty schedule entries from API responses, resulting in cleaner and more efficient data transmission to clients.

@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The Schedule field's JSON tag in the TripDetails struct is updated from json:"schedule" to json:"schedule,omitempty", so when Schedule is a nil pointer the schedule key is omitted entirely from JSON output instead of serializing as null.

Changes

TripDetails schedule omitempty fix

Layer / File(s) Summary
Add omitempty to TripDetails.Schedule JSON tag
internal/models/trip_details.go
TripDetails.Schedule JSON tag gains omitempty, causing the schedule key to be absent from serialized JSON when the pointer is nil.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 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 The title accurately summarizes the main change: adding omitempty to the schedule field to fix JSON serialization behavior.
Linked Issues check ✅ Passed The code change directly addresses issue #1058 by adding the omitempty tag to the Schedule field, ensuring the schedule key is omitted from JSON when nil.
Out of Scope Changes check ✅ Passed The pull request contains only the necessary change to fix the identified issue with no extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

@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

🧹 Nitpick comments (1)
internal/models/trip_details.go (1)

7-7: ⚡ Quick win

Consider adding an inline comment for consistency and documentation.

Line 6 includes an inline comment explaining why omitempty is intentional for the Frequency field. Since this change addresses a specific compliance requirement (OpenOneBusAway wiki spec), adding a similar comment would improve consistency and document the intentional behavior.

📝 Suggested inline comment
-	Schedule     *Schedule   `json:"schedule,omitempty"`
+	Schedule     *Schedule   `json:"schedule,omitempty"` // omitempty intentional: trip-details callers expect the field absent when includeSchedule=false per OpenOneBusAway wiki spec
🤖 Prompt for 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.

In `@internal/models/trip_details.go` at line 7, The Schedule field struct tag on
line 7 lacks an inline comment explaining the intentional use of omitempty,
while the Frequency field on line 6 includes such a comment for consistency and
documentation. Add an inline comment to the Schedule field that explains the
intentional omitempty behavior related to the OpenOneBusAway wiki spec
compliance requirement, following the same comment pattern and style used for
the Frequency field on line 6.
🤖 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/models/trip_details.go`:
- Line 7: The typecheck errors indicate that the types Frequency, Schedule,
ModelTime, Location, and TripStatus are not defined or visible in the current
context, even though they should exist in the internal/models package. Check
whether these types are defined in other Go files within the internal/models
directory; if they are, verify that all relevant files are being compiled
together and there are no circular dependencies or build constraint issues
preventing them from being recognized. If these type definitions are missing
entirely, you must create them in the appropriate files within internal/models.
Review all references to these undefined types throughout the file (particularly
in the TripDetails struct fields) and ensure the type definitions are properly
exported (capitalized names) and accessible to the compiler.

---

Nitpick comments:
In `@internal/models/trip_details.go`:
- Line 7: The Schedule field struct tag on line 7 lacks an inline comment
explaining the intentional use of omitempty, while the Frequency field on line 6
includes such a comment for consistency and documentation. Add an inline comment
to the Schedule field that explains the intentional omitempty behavior related
to the OpenOneBusAway wiki spec compliance requirement, following the same
comment pattern and style used for the Frequency field on line 6.
🪄 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: 368dccf1-0317-4203-9e3d-ac678e8af669

📥 Commits

Reviewing files that changed from the base of the PR and between 9898b8f and 4a693d2.

📒 Files selected for processing (1)
  • internal/models/trip_details.go

Comment thread internal/models/trip_details.go
@3rabiii 3rabiii requested a review from burma-shave June 14, 2026 22:31
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: schedule field emits null instead of being absent when includeSchedule=false

1 participant