-
Notifications
You must be signed in to change notification settings - Fork 1k
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 feed publisher name and url to GTFS GraphQL API #5835
Add feed publisher name and url to GTFS GraphQL API #5835
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #5835 +/- ##
=============================================
+ Coverage 67.91% 68.40% +0.49%
- Complexity 16559 16698 +139
=============================================
Files 1910 1915 +5
Lines 72438 72679 +241
Branches 7447 7453 +6
=============================================
+ Hits 49194 49718 +524
+ Misses 20724 20404 -320
- Partials 2520 2557 +37 ☔ View full report in Codecov by Sentry. |
Can you add a test like this: https://github.com/opentripplanner/OpenTripPlanner/blob/621b802d93a2eb595b645b77be21a36da7314842/src/test/resources/org/opentripplanner/apis/gtfs/queries/alerts.graphql#L1-L0 You have to set up some mock data like this: OpenTripPlanner/src/test/java/org/opentripplanner/apis/gtfs/GraphQLIntegrationTest.java Line 152 in 39fe08e
|
OK I will add the test, thanks for instructions! |
src/test/resources/org/opentripplanner/apis/gtfs/queries/feedinfo.graphql
Outdated
Show resolved
Hide resolved
Data is now structured. |
src/main/java/org/opentripplanner/transit/model/organization/FeedPublisher.java
Outdated
Show resolved
Hide resolved
If you have anyone at hsl that can be second reviewer in Joel's absence, you can assign them. |
type FeedPublisher { | ||
"""Name of feed publisher""" | ||
name: String! | ||
|
||
"""Web address of feed publisher""" | ||
url: String! | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With GTFS these are required but how about NETEX?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that in Netex you don't have a feed publisher at all and you're right: the publisher should be optional. If these two fields should be optional, I'm not sure about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I realised something: you need to also map the FeedPublisher type in graphql-codegen.yml:
OpenTripPlanner/src/main/java/org/opentripplanner/apis/gtfs/generated/graphql-codegen.yml
Line 115 in 6cb475c
StopPosition: org.opentripplanner.apis.gtfs.model.StopPosition#StopPosition |
src/main/resources/org/opentripplanner/apis/gtfs/schema.graphqls
Outdated
Show resolved
Hide resolved
src/main/resources/org/opentripplanner/apis/gtfs/schema.graphqls
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a small doc change.
Co-authored-by: Leonard Ehrenfried <[email protected]>
Out of curiosity, what is this convention of double-double-quotes around comments while single double quotes seem to work as well? |
This allows you to write multi-line doc strings. Regular single-quoted strings have to stay on one line. |
Summary
Add fetchers for FeedInfo publisherName and publisherUrl information.
Unit tests
Feedinfo test query added.
Documentation
In gtfs graphql schema.