Add support for coordinate via point in the GTFS GraphQL API #6490
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #6490 +/- ##
==========================================
Coverage 70.20% 70.20%
- Complexity 18312 18315 +3
==========================================
Files 2080 2083 +3
Lines 77182 77226 +44
Branches 7831 7833 +2
==========================================
+ Hits 54182 54219 +37
- Misses 20231 20235 +4
- Partials 2769 2772 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
optionsome
force-pushed
the
gtfs-api-via-coordinates
branch
from
1cf25c4 to
86707e8
Compare
| private static List<WgsCoordinate> mapCoordinate(@Nullable Object coordinate) { | ||
| if (coordinate == null) { | ||
| return List.of(); | ||
| } | ||
| var map = (Map<String, Object>) coordinate; | ||
| return List.of(new WgsCoordinate((Double) map.get("latitude"), (Double) map.get("longitude"))); | ||
| } |
There was a problem hiding this comment.
Consider putting the Coordinate mapping into a separate class so it can be reused.
There was a problem hiding this comment.
The PlanCoordinateInput is not defined in this PR. If in use there should already be a mapper for it?
There was a problem hiding this comment.
For origin and destination, we map the GraphQL coordinates directly into GenericLocation objects. Here we parse the coordinates into WgsCoordinate objects. However, I think in the future, there will be other use cases for parsing these mapping coordinates to WgsCoordinates so I'll extract the mapping elsewhere.
There was a problem hiding this comment.
Side note, the way how the coordinate is read here is not what I've usually used. Sometimes we have to use this untyped map for parsing the arguments but often it's possible to use the generated types to parse the arguments. @leonardehrenfried originally wrote this class so I'm not sure if we run into some bug or not in this case if we used the generated types.
There was a problem hiding this comment.
Feel free to change it. I vaguely remember that I tried it but there was a problem. Sorry, I cannot remember more.
There was a problem hiding this comment.
Seems to work 902a172 . I think the issue might have been for mapping a list of objects. I didn't use the types for the list of via points.
There was a problem hiding this comment.
The coordinate mapping should be moved out even if there are different ways of doing it. Also, this mapper will then of cause just use the extracted mapping.
Summary
Adds support for coordinate via point in the GTFS GraphQL API.
Issue
Related to #4887 (comment)
Unit tests
Updated GraphQL integration test.
Documentation
None
Changelog
From title