Skip to content

fix: sort schedule-for-stop times by departure time#1037

Open
ARCoder181105 wants to merge 1 commit into
OneBusAway:mainfrom
ARCoder181105:fix/1033-sort-by-departure-time
Open

fix: sort schedule-for-stop times by departure time#1037
ARCoder181105 wants to merge 1 commit into
OneBusAway:mainfrom
ARCoder181105:fix/1033-sort-by-departure-time

Conversation

@ARCoder181105

@ARCoder181105 ARCoder181105 commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR corrects the sorting behavior of the schedule-for-stop API endpoint. Previously, fixed-schedule stop times within direction groups were being sorted sequentially by arrival_time. This has been updated to align with the specification, ensuring they are strictly sorted by departure_time.

Changes Made

  • SQL Queries: Updated GetScheduleForStop and GetScheduleForStopOnDate in gtfsdb/query.sql to use ORDER BY r.id, st.departure_time.
  • Generated Code: Ran make models to regenerate query.sql.go and align it with the updated SQL.
  • Unit Tests: Enhanced TestScheduleForStopQueryValidation in internal/restapi/schedule_for_stop_handler_test.go with an explicit assertion loop to verify that stop times are strictly returned in ascending order of departureTime.

Acceptance Criteria Verified

  • Stop times within a StopRouteDirectionSchedule are sorted strictly by departure_time ascending.
  • Unit tests verify this sorting behavior explicitly.

Closes #1033

Summary by CodeRabbit

  • Bug Fixes

    • Updated schedule query results to order by departure time instead of arrival time.
  • Tests

    • Added validation that schedule results are correctly sorted by departure time.

@coderabbitai

coderabbitai Bot commented Jun 6, 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: 80eb2f59-b4d0-4434-849b-798a07227f71

📥 Commits

Reviewing files that changed from the base of the PR and between e99ee63 and 0b31450.

📒 Files selected for processing (3)
  • gtfsdb/query.sql
  • gtfsdb/query.sql.go
  • internal/restapi/schedule_for_stop_handler_test.go

📝 Walkthrough

Walkthrough

The PR fixes the schedule stop times sorting order by updating both SQL queries and their Go-generated equivalents to sort by departure_time instead of arrival_time, and adds validation tests to confirm the fix works correctly.

Changes

Schedule Sort Order Fix

Layer / File(s) Summary
Schedule query sort order fix
gtfsdb/query.sql, gtfsdb/query.sql.go
The GetScheduleForStop and GetScheduleForStopOnDate queries are updated to order by r.id, st.departure_time instead of r.id, st.arrival_time. Both the SQL source and generated Go code reflect this change.
Departure time sorting validation
internal/restapi/schedule_for_stop_handler_test.go
TestScheduleForStopQueryValidation validates that stopTimes within the returned schedule are sorted in ascending order by departureTime through adjacent-pair comparisons.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested reviewers

  • burma-shave
  • aaronbrethorst
  • Ahmedhossamdev
🚥 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: modifying the sort order of schedule-for-stop times from arrival_time to departure_time, which is the core fix addressed in the PR.
Linked Issues check ✅ Passed The PR fully addresses issue #1033: SQL queries updated to sort by departure_time, Go code regenerated, and unit tests enhanced to verify departure_time sorting.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #1033: SQL query ordering fixes, code regeneration, and test enhancements for departure_time sorting.
Docstring Coverage ✅ Passed Docstring coverage is 100.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 and usage tips.

@sonarqubecloud

sonarqubecloud Bot commented Jun 6, 2026

Copy link
Copy Markdown

@ARCoder181105

Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@ARCoder181105 ARCoder181105 requested a review from burma-shave June 8, 2026 03:11
@ARCoder181105

Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

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.

schedule-for-stop: Stop times are sorted by arrival time instead of departure time

1 participant