Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a "canceled" field to Place in GTFS GraphQL API #6445

Draft
wants to merge 4 commits into
base: dev-2.x
Choose a base branch
from

Conversation

miklcct
Copy link
Contributor

@miklcct miklcct commented Feb 7, 2025

Summary

This adds a canceled field into Place in GTFS GraphQL API, which is referenced in Leg.

It allows clients to show to the rider that if the boarding call, alighting call, or any of the intermediate calls are canceled.

It is possible to see if the boarding call or alighting call is canceled now indirectly by using pickupType and dropoffType in Leg (if the leg is a transit leg and it is null, it means that the call is canceled, but this is not documented at the API level and is only inferred by reading the code), but not intermediate calls.

Issue

#6441

Unit tests

I have added a unit test for intermediate calls.

Documentation

Added in GraphQL

Copy link

codecov bot commented Feb 7, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 70.09%. Comparing base (a7c5aec) to head (5eb4dca).

Files with missing lines Patch % Lines
...pentripplanner/apis/gtfs/datafetchers/LegImpl.java 66.66% 0 Missing and 2 partials ⚠️
...entripplanner/apis/gtfs/datafetchers/PlanImpl.java 0.00% 2 Missing ⚠️
...pentripplanner/model/plan/FrequencyTransitLeg.java 0.00% 2 Missing ⚠️
.../opentripplanner/model/plan/StopArrivalMapper.java 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #6445      +/-   ##
=============================================
- Coverage      70.10%   70.09%   -0.01%     
- Complexity     18269    18270       +1     
=============================================
  Files           2077     2077              
  Lines          77659    77667       +8     
  Branches        7828     7834       +6     
=============================================
+ Hits           54439    54442       +3     
- Misses         20439    20441       +2     
- Partials        2781     2784       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@miklcct miklcct marked this pull request as ready for review February 11, 2025 15:48
@miklcct miklcct requested a review from a team as a code owner February 11, 2025 15:48
@optionsome
Copy link
Member

Perhaps LegTime would be a more fitting place for this information rather than directly on Place. The reason is that in theory, departure from a stop might be canceled while arrival is still scheduled because the last stop(s) of the trip are canceled. Also, maybe we should differentiate between "planned cancellations" and "real-time cancellations". For that, there are two options: either make the boolean field's name more specific or some sort of an enum. I think we have previously identified some issues with trying to combine too many things into one enum as a trip might be both added and cancelled, for example. So maybe I'm leaning towards a boolean field called isRealTimeCanceled on the LegTime.

We could discuss this in a dev meeting next week.

@miklcct
Copy link
Contributor Author

miklcct commented Feb 14, 2025

GTFS does not support planned cancellations and I have no idea how it works internally with Transmodel.

Also, I don't think that it is possible to cancel the arrival but not departure even if the trip is terminated short in the current model.

@t2gran t2gran added this to the Parked milestone Feb 18, 2025
@t2gran t2gran marked this pull request as draft February 18, 2025 10:18
@optionsome
Copy link
Member

We decided to do a bigger refactoring after 2.7. For now, the pickup/dropoff statuses can be used to interpret that a stop is cancelled/skipped. We are parking this pr for now.

@optionsome optionsome removed their request for review February 24, 2025 12:19
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.

3 participants