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 headway to GTFS GraphQL API #5951

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

Conversation

heeleeaz
Copy link

@heeleeaz heeleeaz commented Jul 6, 2024

Summary

This PR adds headway to Plan Itinerary.Leg

Issue

#5950

Unit tests

No Unit test required

Documentation

  • Documentation is included in the GraphQL model.

@heeleeaz heeleeaz requested a review from a team as a code owner July 6, 2024 17:12
@leonardehrenfried leonardehrenfried changed the title Leg headway graphql Add headway to GTFS GraphQL API Jul 8, 2024
non-strict frequency trips, but could become important for real-time trips, strict frequency
trips, and scheduled trips with empirical headways.
"""
headway: Int
Copy link
Member

Choose a reason for hiding this comment

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

Can you return a Duration here? We already have the scalar for it.

@@ -163,6 +165,15 @@ public static GraphQLObjectType create(
.dataFetcher(env -> leg(env).getDuration().toSeconds())
.build()
)
.field(
Copy link
Member

Choose a reason for hiding this comment

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

Are you using this API or the GTFS one? If you don't use it, you don't need to add this code here.

Copy link

codecov bot commented Jul 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.46%. Comparing base (bd2eced) to head (2eaf8ea).
Report is 3 commits behind head on dev-2.x.

Additional details and impacted files
@@            Coverage Diff             @@
##             dev-2.x    #5951   +/-   ##
==========================================
  Coverage      69.46%   69.46%           
+ Complexity     17075    17074    -1     
==========================================
  Files           1937     1937           
  Lines          73717    73725    +8     
  Branches        7546     7546           
==========================================
+ Hits           51204    51211    +7     
  Misses         19882    19882           
- Partials        2631     2632    +1     

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

@leonardehrenfried
Copy link
Member

Please also add a test for the new field.

  1. Add it to the query here: https://github.com/opentripplanner/OpenTripPlanner/blob/12c51f44f204db31d34a1eeb0d59204226e0fa5d/src/test/resources/org/opentripplanner/apis/gtfs/queries/plan-extended.graphql#L1-L0
  2. Fill the data set with a frequency leg here:
    Itinerary i1 = newItinerary(A, T11_00)
    .walk(20, B, List.of(step1, step2))
    .bus(busRoute, 122, T11_01, T11_15, C)
    .rail(439, T11_30, T11_50, D)
    .carHail(D10m, E)
    .build();

@t2gran t2gran added this to the 2.6 (next release) milestone Jul 23, 2024
@leonardehrenfried
Copy link
Member

I'm going to convert this to a draft. Once you start working on it again, you can turn it back into a PR.

@leonardehrenfried leonardehrenfried marked this pull request as draft July 25, 2024 12:32
@t2gran t2gran modified the milestones: 2.6, 2.7 (next release) Sep 18, 2024
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