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

Expose station entrances in GraphQL API #6082

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

Conversation

cedarbaum
Copy link
Contributor

Summary

This change exposes GTFS station entrances via the GraphQL API. It provides a new top-level entrances query which returns all station entrances as well as a new entrances subquery of the stations query which returns all entrances associated with a station.

Top-level query

entrances() {
  gtfsId
  lat
  lon
  name
  vehicleMode
  locationType
}

stations subquery

stations {
  gtfsId
  lat
  lon
  name
  stops {
    ...
  }
  entrances {
    gtfsId
    lat
    lon
    name
    vehicleMode
    locationType
  }
}

Issue

This is a proposed change as part of issue #6081. Further discussion of the implementation and API are still pending as part of this issue, but this PR hopefully will provide additional context. It is possible this PR will evolve significantly with this issue - if it makes more sense to make it a draft, I can do so.

Unit tests

  • Unit tests were added for associating Entrance objects with their parent station
  • GraphQL integration tests were added for the new queries.

Documentation

  • Documentation for new GraphQL queries has been added.

Copy link

codecov bot commented Sep 20, 2024

Codecov Report

Attention: Patch coverage is 66.95652% with 38 lines in your changes missing coverage. Please review.

Project coverage is 69.81%. Comparing base (b108104) to head (a829afb).

Files with missing lines Patch % Lines
...entripplanner/apis/gtfs/datafetchers/StopImpl.java 45.90% 32 Missing and 1 partial ⚠️
...ntripplanner/apis/gtfs/generated/GraphQLTypes.java 54.54% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #6082      +/-   ##
=============================================
+ Coverage      69.78%   69.81%   +0.02%     
- Complexity     17357    17400      +43     
=============================================
  Files           1962     1962              
  Lines          74357    74450      +93     
  Branches        7623     7629       +6     
=============================================
+ Hits           51893    51978      +85     
- Misses         19820    19823       +3     
- Partials        2644     2649       +5     

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

@leonardehrenfried
Copy link
Member

We need to harmonise our approaches as #6076 does something similar.

@optionsome
Copy link
Member

@HenrikSundell will do implement a entrance type to the graphql schema in a different pr soon-ish, and you can then use that type for the entrances in this pr instead of using the stop type.

@cedarbaum
Copy link
Contributor Author

Thanks for the update @optionsome! Are we all aligned on returning both OSM and GTFS provided entrances in a common type? If so, it sounds like the remaining work on this PR will be to integrate with the OSM entrance data.

@HenrikSundell please let me know if there is anything I can do to help or if you want to chat about synchronizing our changes.

@optionsome
Copy link
Member

Are we all aligned on returning both OSM and GTFS provided entrances in a common type? If so, it sounds like the remaining work on this PR will be to integrate with the OSM entrance data.

Yes you are correct, we will use same type for both.

@t2gran t2gran added this to the 2.7 (next release) milestone Oct 16, 2024
@leonardehrenfried leonardehrenfried marked this pull request as draft November 25, 2024 07:05
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.

4 participants