feat: Smart/Origin: ffcam livraison1#4569
Open
lgourdin wants to merge 31 commits into
Open
Conversation
added 19 commits
July 3, 2026 09:29
…ints of type access and hut ± Conflicts: ± src/views/document/WaypointView.vue
…e return trip functionnality
WalkthroughThis PR adds a public-transport trip-planning flow, splits the planning UI into form/results/orchestration components, updates access-waypoint handling and document wiring, fixes address selection state, and expands French trip-planning translations. ChangesPublic transport trip planning refactor
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant PlanATripSection
participant NavitiaService
participant MapView
User->>PlanATripSection: calculateRoute()
PlanATripSection->>NavitiaService: getJourneys(datetime -15min)
NavitiaService-->>PlanATripSection: journeys
alt journeys match selected date
PlanATripSection->>PlanATripSection: filter journeys, determineReturnWaypoint()
else no match found
PlanATripSection->>NavitiaService: fetchExtendedTimeframeJourney()
NavitiaService-->>PlanATripSection: extended journeys
end
PlanATripSection->>PlanATripSection: prepareRouteDocuments()
PlanATripSection->>MapView: render filteredDocuments
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (6)
src/js/plan-a-trip-utils.js (1)
20-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
getTransportIconandgetTransportClassare byte-for-byte identical.Both functions contain the exact same mode-matching logic and always return the same string. Keeping two separate copies risks silent divergence the next time a transport mode is added/changed (icon and CSS class could end up mismatched).
♻️ Suggested consolidation
- /** Gets icons according to the nature of the transport */ - getTransportIcon(section) { - if (!section.display_informations) return ''; - - const mode = section.display_informations.commercial_mode?.toLowerCase() || ''; - if (mode.includes('bus')) return 'bus'; - if (mode.includes('tram')) return 'tram'; - if (mode.includes('métro') || mode.includes('metro')) return 'tram'; - if (mode.includes('train')) return 'train'; - if (mode.includes('car')) return 'bus'; - - return 'bus'; - }, - - /** Manage different spellings of transports */ - getTransportClass(section) { - if (!section.display_informations) return ''; - - const mode = section.display_informations.commercial_mode?.toLowerCase() || ''; - - if (mode.includes('bus')) return 'bus'; - if (mode.includes('tram')) return 'tram'; - if (mode.includes('métro') || mode.includes('metro')) return 'tram'; - if (mode.includes('train')) return 'train'; - if (mode.includes('car')) return 'bus'; - - return 'bus'; - }, + /** Normalizes commercial mode into a canonical transport key, used for both icon file name and CSS class */ + getTransportKey(section) { + if (!section.display_informations) return ''; + const mode = section.display_informations.commercial_mode?.toLowerCase() || ''; + if (mode.includes('bus') || mode.includes('car')) return 'bus'; + if (mode.includes('tram') || mode.includes('métro') || mode.includes('metro')) return 'tram'; + if (mode.includes('train')) return 'train'; + return 'bus'; + }, + getTransportIcon(section) { + return this.getTransportKey(section); + }, + getTransportClass(section) { + return this.getTransportKey(section); + },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/plan-a-trip-utils.js` around lines 20 - 47, `getTransportIcon` and `getTransportClass` in plan-a-trip-utils.js are identical and should be consolidated to a single source of truth. Extract the shared mode-to-value mapping into one helper used by both functions, or have one delegate to the other, so transport icon and CSS class logic in `getTransportIcon`/`getTransportClass` cannot drift apart when adding or changing modes.src/views/document/utils/boxes/PlanATripSection/PlanATripSection.vue (2)
1125-1219: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLeftover dead CSS from the component split.
.itineraries-container/.itinerary-header/.selectedare re-declared nested inside.no-itineraries-containerhere, but that markup was moved toPlanATripResults.vue(which has its ownscopedcopy of the same rules, Lines 260-303 there). Since Vuescopedstyles never cross component boundaries, and this file's own template no longer renders an.itineraries-containerelement, this nested block is now unreachable/dead.♻️ Suggested cleanup
.no-itineraries-container { border: 1px solid lightgrey; position: relative; border-radius: 4px; max-width: 450px; .no-itineraries { ... } - .itineraries-container { - max-width: 490px; - flex: 1; - padding-right: 16px; - transition: background-color 0.3s ease; - position: relative; - - .itinerary-header { - ... - } - .selected { - border-left: 4px solid `#4baf50`; - background-color: `#fbfaf6`; - } - } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/document/utils/boxes/PlanATripSection/PlanATripSection.vue` around lines 1125 - 1219, Remove the dead styling block from PlanATripSection.vue: the nested `.itineraries-container`, `.itinerary-header`, and `.selected` rules inside `.no-itineraries-container` are no longer used after the split. Keep the `.no-itineraries-container` and `.navitia-unknown-error` styles here, and leave the active itineraries styling in PlanATripResults.vue where the matching markup now lives.
946-987: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRemove the dead Case 5 branch The earlier checks already return for every possible
reachableWaypoints.length, so this fallback can never run. If the outbound-waypoint fallback is still needed, move it before thelength > 2return.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/document/utils/boxes/PlanATripSection/PlanATripSection.vue` around lines 946 - 987, The fallback branch in determineReturnWaypoint is dead code because all reachableWaypoints length cases already return before it can execute. Remove the unreachable “Case 5” block, or if outboundData.selectedWaypoint should still be reused when reachableWaypoints contains it, move that logic earlier in determineReturnWaypoint before the length > 2 return so it can actually run.src/components/generics/inputs/InputAddress.vue (1)
226-232: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMutates the selected
propositionobject in place instead of copying it.
localData.selectedAddress = propositionfollowed bylocalData.selectedAddress.address = ...mutates the original object reference (e.g. a Photon GeoJSON feature) rather than creating a new object. If this same object reference is retained elsewhere (e.g. still present inaddressPropositions, or reused by a caller), the added.addressfield silently pollutes it. A shallow copy avoids this risk.💡 Suggested fix
selectAddress(proposition) { this.localData.address = this.formatProposition(proposition); - this.localData.selectedAddress = proposition; - this.localData.selectedAddress.address = this.localData.address; + this.localData.selectedAddress = { ...proposition, address: this.localData.address }; this.localData.coordinates = proposition.geometry.coordinates; this.localData.showAddressPropositions = false; },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/generics/inputs/InputAddress.vue` around lines 226 - 232, The selectAddress method is mutating the incoming proposition object by assigning it directly to localData.selectedAddress and then writing address onto it. Update selectAddress in InputAddress.vue to create a shallow copy of proposition before storing or enriching it, so the original GeoJSON feature/object is not polluted. Keep the rest of the flow the same: format the address, preserve geometry.coordinates, and still hide the propositions list.src/views/document/utils/boxes/TransportsBox.vue (1)
105-117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
accessWaypointslogic across three files.This exact branching (
documentType === 'waypoint' && waypoint_type === 'access'→[document], else filterassociations.waypoints) is copy-pasted near-verbatim inIsReachableByPublicTransportsBox.vue,NearbyStopsSection.vue, and here. Consider extracting a shared helper (e.g. a mixin or a function inplan-a-trip-utils.js) such asgetAccessWaypoints(document, documentType)to avoid drift as this logic evolves (it already needed a bugfix in all three places, see the sibling comment).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/document/utils/boxes/TransportsBox.vue` around lines 105 - 117, The access-waypoints branching is duplicated and should be centralized to avoid future drift. Extract the shared logic from this component’s accessWaypoints handling into a reusable helper such as getAccessWaypoints(document, documentType) in plan-a-trip-utils.js (or a shared mixin/util), then update TransportsBox.vue, IsReachableByPublicTransportsBox.vue, and NearbyStopsSection.vue to call it instead of inlining the same waypoint/access filter condition.src/views/document/WaypointView.vue (1)
169-182: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider centralizing
TRANSPORT_BOX_TYPES.This allowlist is defined inline here and conceptually related to
TransportsBox.vue'sisPlanATripSectionDisplayedwaypoint-type checks. Extracting it to a shared constant (e.g. alongsideplan-a-trip-utils.js) would reduce risk of the two lists silently diverging as waypoint types are added/removed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/document/WaypointView.vue` around lines 169 - 182, The waypoint-type allowlist is duplicated inline in showTransportBox and overlaps with TransportsBox.vue’s isPlanATripSectionDisplayed checks, so move TRANSPORT_BOX_TYPES into a shared constant used by both places. Define it in a common utility such as plan-a-trip-utils.js, import it in WaypointView.vue and TransportsBox.vue, and update both checks to reference the shared source so the lists cannot drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/js/plan-a-trip-utils.js`:
- Around line 105-125: The calculateSectionDuration helper in plan-a-trip-utils
is recomputing duration from departure_date_time and arrival_date_time, which
ignores the date and can produce negative results for overnight sections. Update
calculateSectionDuration to use section.duration directly instead of parsing the
timestamps, and keep the fallback behavior returning 0 when duration is
unavailable.
In `@src/views/document/utils/boxes/IsReachableByPublicTransportsBox.vue`:
- Around line 35-41: In IsReachableByPublicTransportsBox.vue, the
accessWaypoints lookup in the related computed logic can return undefined when
associations.waypoints is missing, which then breaks checkAccessibility() when
it reads length. Update the branch that returns the filtered waypoints so it
always yields an array, and keep the waypoint access special-case intact. Use
the existing documentType/document.waypoint_type handling and the
checkAccessibility() caller as the reference points while adding the fallback.
In `@src/views/document/utils/boxes/NearbyStopsSection.vue`:
- Around line 129-141: The fallback path in NearbyStopsSection’s access-waypoint
selection can leave accessWaypoints undefined, which then breaks the
JSON.parse(JSON.stringify(...)) clone. Update the computed logic around the
accessWaypoints assignment to default the filtered associations to an empty
array (for example with a nullish fallback) or bypass the deep clone when no
items exist, so the property remains safe when a document has no access
waypoints.
In `@src/views/document/utils/boxes/PlanATripSection/PlanATripForm.vue`:
- Around line 51-81: The return-trip destination field in PlanATripForm.vue is
using the wrong placeholder text, since the input-address bound to
currentData.fromAddress is the “To” address when activeTab is return. Update
that placeholder to a destination/arrival-oriented string instead of “Enter a
departure address,” and keep the change localized to the return-trip branch so
the outbound field remains unchanged.
In `@src/views/document/utils/boxes/PlanATripSection/PlanATripResults.vue`:
- Line 70: The fallback labels in PlanATripResults.vue are hardcoded and bypass
localization. Update the affected template expressions in the PlanATripResults
component so the fallback values for the waypoint/title and timeline labels use
$gettext instead of raw strings. Replace the untranslated "Destination" as well
as the hardcoded French "Départ" and "Arrivée" with translated fallbacks in the
same places where currentData.selectedWaypoint and the timeline label rendering
are handled.
- Around line 29-35: The transport icon markup in PlanATripResults.vue is using
the wrong alt text, with the public_transport/on_demand_transport image still
labeled as walking. Update the img element in the transport-icon block to use an
alt value that matches the transport icon being rendered, and keep the walking
alt text only in the walking-icon branch so assistive technology gets the
correct label.
In `@src/views/document/utils/boxes/PlanATripSection/PlanATripSection.vue`:
- Around line 270-272: Fix the typo in the source string used by
PlanATripSection so the unknown_error gettext key uses “occurred” instead of
“occured”; update the matching translation entry in fr.json at the same key so
the source string and translation lookup stay in sync. Use the existing
unknown_error and navitia_internal_error entries in PlanATripSection.vue to
locate the change.
- Around line 317-346: mapDocuments is doing more than computing data because it
calls prepareRouteDocuments, which schedules recenterOnDocuments via $nextTick.
Move the map recenter side effect out of the computed chain and keep
mapDocuments pure; instead trigger the recenter from a watch on
currentData.selectedRouteJourney or immediately after
calculateRoute/fetchExtendedTimeframeJourney completes. Update
prepareRouteDocuments and the $refs.mapView.recenterOnDocuments call so they are
only reached from the new side-effect path, not from the computed getter.
- Around line 906-943: Make loadReachableWaypoints in PlanATripSection handle
missing associations safely by defaulting accessPoints to an empty array instead
of allowing this.document?.associations?.waypoints?.filter(...) to produce
undefined, so the for...of over accessPoints never crashes. Also wrap the
transportService.isReachable/PROMISE.all flow in try/catch or equivalent error
handling, and ensure loadingReachable is reset to false in a finally block so a
failed reachability check does not leave the UI stuck loading.
- Around line 477-528: The `fetchExtendedTimeframeJourney` fallback in
`PlanATripSection.vue` is currently triggered for every failure in the
`try/catch`, even for non-retryable Navitia errors. Update the `catch` in the
journey fetch flow to only call `fetchExtendedTimeframeJourney` for retryable
cases, and skip it for `auth_error`, `invalid_param`, `navitia_internal_error`,
or other unknown/non-recoverable errors. Use the existing `queryError` parsing
from `error?.response?.data?.errors?.[0]?.description` and the journey-fetch
methods (`getJourneys`, `fetchExtendedTimeframeJourney`) to gate the fallback
instead of unconditionally retrying.
In `@src/views/document/utils/boxes/TransportsBox.vue`:
- Around line 105-117: The waypoint filtering logic in TransportsBox.vue can
still leave accessWaypoints undefined when document.associations.waypoints is
missing, which then breaks the JSON.parse(JSON.stringify(...)) copy step. Update
the accessWaypoints assignment in the TransportsBox component to default the
filtered result to an empty array using nullish coalescing, so the later
accessPointsCopy clone always receives an array.
---
Nitpick comments:
In `@src/components/generics/inputs/InputAddress.vue`:
- Around line 226-232: The selectAddress method is mutating the incoming
proposition object by assigning it directly to localData.selectedAddress and
then writing address onto it. Update selectAddress in InputAddress.vue to create
a shallow copy of proposition before storing or enriching it, so the original
GeoJSON feature/object is not polluted. Keep the rest of the flow the same:
format the address, preserve geometry.coordinates, and still hide the
propositions list.
In `@src/js/plan-a-trip-utils.js`:
- Around line 20-47: `getTransportIcon` and `getTransportClass` in
plan-a-trip-utils.js are identical and should be consolidated to a single source
of truth. Extract the shared mode-to-value mapping into one helper used by both
functions, or have one delegate to the other, so transport icon and CSS class
logic in `getTransportIcon`/`getTransportClass` cannot drift apart when adding
or changing modes.
In `@src/views/document/utils/boxes/PlanATripSection/PlanATripSection.vue`:
- Around line 1125-1219: Remove the dead styling block from
PlanATripSection.vue: the nested `.itineraries-container`, `.itinerary-header`,
and `.selected` rules inside `.no-itineraries-container` are no longer used
after the split. Keep the `.no-itineraries-container` and
`.navitia-unknown-error` styles here, and leave the active itineraries styling
in PlanATripResults.vue where the matching markup now lives.
- Around line 946-987: The fallback branch in determineReturnWaypoint is dead
code because all reachableWaypoints length cases already return before it can
execute. Remove the unreachable “Case 5” block, or if
outboundData.selectedWaypoint should still be reused when reachableWaypoints
contains it, move that logic earlier in determineReturnWaypoint before the
length > 2 return so it can actually run.
In `@src/views/document/utils/boxes/TransportsBox.vue`:
- Around line 105-117: The access-waypoints branching is duplicated and should
be centralized to avoid future drift. Extract the shared logic from this
component’s accessWaypoints handling into a reusable helper such as
getAccessWaypoints(document, documentType) in plan-a-trip-utils.js (or a shared
mixin/util), then update TransportsBox.vue,
IsReachableByPublicTransportsBox.vue, and NearbyStopsSection.vue to call it
instead of inlining the same waypoint/access filter condition.
In `@src/views/document/WaypointView.vue`:
- Around line 169-182: The waypoint-type allowlist is duplicated inline in
showTransportBox and overlaps with TransportsBox.vue’s
isPlanATripSectionDisplayed checks, so move TRANSPORT_BOX_TYPES into a shared
constant used by both places. Define it in a common utility such as
plan-a-trip-utils.js, import it in WaypointView.vue and TransportsBox.vue, and
update both checks to reference the shared source so the lists cannot drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2db78652-89b3-4626-99b4-b6ac7f1b9d92
⛔ Files ignored due to path filters (1)
src/assets/img/boxes/navitia_api_error.svgis excluded by!**/*.svg
📒 Files selected for processing (12)
src/components/generics/inputs/InputAddress.vuesrc/js/plan-a-trip-utils.jssrc/translations/fr.jsonsrc/views/document/RouteView.vuesrc/views/document/WaypointView.vuesrc/views/document/utils/boxes/IsReachableByPublicTransportsBox.vuesrc/views/document/utils/boxes/NearbyStopsSection.vuesrc/views/document/utils/boxes/PlanATripSection.vuesrc/views/document/utils/boxes/PlanATripSection/PlanATripForm.vuesrc/views/document/utils/boxes/PlanATripSection/PlanATripResults.vuesrc/views/document/utils/boxes/PlanATripSection/PlanATripSection.vuesrc/views/document/utils/boxes/TransportsBox.vue
💤 Files with no reviewable changes (1)
- src/views/document/utils/boxes/PlanATripSection.vue
Comment on lines
+270
to
+272
| // internal server error | ||
| unknown_error: this.$gettext('An unknown error occured.'), | ||
| navitia_internal_error: this.$gettext('Public transport data is currently unavailable.'), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Typo: "occured" → "occurred".
this.$gettext('An unknown error occured.') has a spelling error. Since translation lookups match on the exact source string, this needs to be fixed here and the corresponding key updated in src/translations/fr.json (Line 63) to keep the translation matching.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/views/document/utils/boxes/PlanATripSection/PlanATripSection.vue` around
lines 270 - 272, Fix the typo in the source string used by PlanATripSection so
the unknown_error gettext key uses “occurred” instead of “occured”; update the
matching translation entry in fr.json at the same key so the source string and
translation lookup stay in sync. Use the existing unknown_error and
navitia_internal_error entries in PlanATripSection.vue to locate the change.
added 8 commits
July 6, 2026 09:50
…tbound "From" field.
…n-retryable ones.
added 4 commits
July 6, 2026 09:50
…y checks have no error handling.
…y when no return trip
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/js/plan-a-trip-utils.js (2)
9-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated duration-formatting logic between
formatDurationandformatJourneyDuration.Both compute hours/minutes from seconds with near-identical logic, differing only in display format (
"X h Y min"vs"XhYY"). Consider extracting a sharedhours/minuteshelper to avoid divergence if the formatting rules need to change later.Also applies to: 92-103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/plan-a-trip-utils.js` around lines 9 - 18, The duration formatting logic is duplicated between formatDuration and formatJourneyDuration, so extract the shared hours/minutes calculation into a helper and have both methods reuse it. Keep the formatting differences in each method, but centralize the seconds-to-hours/minutes conversion to avoid future divergence. Use the existing formatDuration and formatJourneyDuration symbols to locate the duplicated block and update both callers to rely on the new shared helper.
20-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
getTransportIconandgetTransportClassare identical implementations.Both functions have the exact same body and will always return the same value for any given section. If the icon key and CSS class are meant to be able to diverge (e.g. more granular classes later), this coupling will silently break; if they're truly meant to be identical, consolidate into one function to avoid future drift.
Also note both fall back to
'bus'for any unmatchedcommercial_mode(e.g. Navitia'sBoat,Ferry,Funicular,Air,Taxiphysical modes), which will mislabel those transport types as bus icons/classes.♻️ Suggested consolidation
- /** Gets icons according to the nature of the transport */ - getTransportIcon(section) { - if (!section.display_informations) return ''; - - const mode = section.display_informations.commercial_mode?.toLowerCase() || ''; - if (mode.includes('bus')) return 'bus'; - if (mode.includes('tram')) return 'tram'; - if (mode.includes('métro') || mode.includes('metro')) return 'tram'; - if (mode.includes('train')) return 'train'; - if (mode.includes('car')) return 'bus'; - - return 'bus'; - }, - - /** Manage different spellings of transports */ - getTransportClass(section) { - if (!section.display_informations) return ''; - - const mode = section.display_informations.commercial_mode?.toLowerCase() || ''; - - if (mode.includes('bus')) return 'bus'; - if (mode.includes('tram')) return 'tram'; - if (mode.includes('métro') || mode.includes('metro')) return 'tram'; - if (mode.includes('train')) return 'train'; - if (mode.includes('car')) return 'bus'; - - return 'bus'; - }, + /** Gets icons and CSS class according to the nature of the transport */ + getTransportIcon(section) { + if (!section.display_informations) return ''; + + const mode = section.display_informations.commercial_mode?.toLowerCase() || ''; + if (mode.includes('bus')) return 'bus'; + if (mode.includes('tram')) return 'tram'; + if (mode.includes('métro') || mode.includes('metro')) return 'tram'; + if (mode.includes('train')) return 'train'; + if (mode.includes('car')) return 'bus'; + + return 'bus'; + }, + + /** Alias kept for template compatibility */ + getTransportClass(section) { + return this.getTransportIcon(section); + },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/plan-a-trip-utils.js` around lines 20 - 47, getTransportIcon and getTransportClass currently duplicate the same transport-mapping logic, so either consolidate the shared behavior into a single helper used by both methods or clearly separate them if icon and CSS class mappings need to diverge later. Update the mapping in plan-a-trip-utils.js around getTransportIcon/getTransportClass to avoid drift, and expand the fallback handling so unmatched commercial_mode values are not mislabeled as bus; add explicit cases for other Navitia modes like Boat, Ferry, Funicular, Air, and Taxi (or a safe default) in the shared transport resolution logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/js/plan-a-trip-utils.js`:
- Around line 9-18: The duration formatting logic is duplicated between
formatDuration and formatJourneyDuration, so extract the shared hours/minutes
calculation into a helper and have both methods reuse it. Keep the formatting
differences in each method, but centralize the seconds-to-hours/minutes
conversion to avoid future divergence. Use the existing formatDuration and
formatJourneyDuration symbols to locate the duplicated block and update both
callers to rely on the new shared helper.
- Around line 20-47: getTransportIcon and getTransportClass currently duplicate
the same transport-mapping logic, so either consolidate the shared behavior into
a single helper used by both methods or clearly separate them if icon and CSS
class mappings need to diverge later. Update the mapping in plan-a-trip-utils.js
around getTransportIcon/getTransportClass to avoid drift, and expand the
fallback handling so unmatched commercial_mode values are not mislabeled as bus;
add explicit cases for other Navitia modes like Boat, Ferry, Funicular, Air, and
Taxi (or a safe default) in the shared transport resolution logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b4580ce3-ba09-4add-a6dd-b56bf71c9dbc
📒 Files selected for processing (8)
src/js/plan-a-trip-utils.jssrc/translations/fr.jsonsrc/views/document/utils/boxes/IsReachableByPublicTransportsBox.vuesrc/views/document/utils/boxes/NearbyStopsSection.vuesrc/views/document/utils/boxes/PlanATripSection/PlanATripForm.vuesrc/views/document/utils/boxes/PlanATripSection/PlanATripResults.vuesrc/views/document/utils/boxes/PlanATripSection/PlanATripSection.vuesrc/views/document/utils/boxes/TransportsBox.vue
✅ Files skipped from review due to trivial changes (1)
- src/translations/fr.json
🚧 Files skipped from review as they are similar to previous changes (6)
- src/views/document/utils/boxes/PlanATripSection/PlanATripForm.vue
- src/views/document/utils/boxes/IsReachableByPublicTransportsBox.vue
- src/views/document/utils/boxes/PlanATripSection/PlanATripResults.vue
- src/views/document/utils/boxes/TransportsBox.vue
- src/views/document/utils/boxes/NearbyStopsSection.vue
- src/views/document/utils/boxes/PlanATripSection/PlanATripSection.vue
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details:
"hut", "gite", "climbing_outdoor", "climbing_indoor", "paragliding_landing", "shelter", "bivouac", "camp_site", "access"
Summary by CodeRabbit