Add alternate route names to itin description #1406
Conversation
| const alternateRouteNames = | ||
| leg.alternateRoutes && | ||
| Object.entries(leg.alternateRoutes).map( | ||
| (route) => route[1].routeShortName |
There was a problem hiding this comment.
Route short name is sometimes not specified! This should be an optional
There was a problem hiding this comment.
Added a filter to this list to match the one on line 74
There was a problem hiding this comment.
This is good but not what I was talking about! route[1] might not contain routeShortName which is the issue
There was a problem hiding this comment.
Okay, added a fallback to this line
| const alternateRouteNames = | ||
| leg.alternateRoutes && | ||
| getAlternateRoutesFromLeg(leg) | ||
| .map((route) => route[1].routeShortName || undefined) |
There was a problem hiding this comment.
This line makes me feel uncomfortable. Without optionals this feels like a disaster waiting to happen... Can we add some optionals here instead of the || undefined?
There was a problem hiding this comment.
That's fair, I was keeping the function consistent, but I agree with you it doesn't make sense to have routes without short names not represented in the list. I added a fallback for the alternate routes but also the default route below.
There was a problem hiding this comment.
Great! We should maybe throw a comment in here saying that this could all be cleaned up in the future... Probably when we implement nextLegs!
Description:
Adds the alternative route to the itinerary description in the case that itineraries are merged
PR Checklist: