Smart origin/lot1#4357
Conversation
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
WalkthroughThis update introduces public transport accessibility features throughout the application. New Vue components display and manage public transport stop information for routes and waypoints, including map integration, highlighting, and proximity warnings. Supporting API services, translation strings, and style adjustments are added. Configuration changes set new API endpoints and local development defaults. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RouteView/WaypointView
participant IsReachableByPublicTransportsBox
participant TransportsBox
participant TransportService
participant Map (OlMap.vue)
User->>RouteView/WaypointView: Load document view
RouteView/WaypointView->>IsReachableByPublicTransportsBox: Pass waypoints as prop
IsReachableByPublicTransportsBox->>TransportService: isReachable(waypointId)
TransportService-->>IsReachableByPublicTransportsBox: Reachability result
IsReachableByPublicTransportsBox-->>User: Show/hide accessibility info
RouteView/WaypointView->>TransportsBox: Pass waypoints as prop
TransportsBox->>TransportService: getStopareasForDocuments(documents)
TransportService-->>TransportsBox: Stop areas data
TransportsBox-->>User: Show stops, warnings, details
User->>TransportsBox: Select stop/stop group
TransportsBox->>Map (OlMap.vue): highlightStop(stopId), goAndZoomOnStop(stopId)
Map (OlMap.vue)-->>TransportsBox: Event feedback (stop-clicked)
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn EBADENGINE Unsupported engine { ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (6)
src/components/map/map-utils.js (1)
62-66: Consider extracting SVG fill logic for better readabilityThe conditional SVG fill replacement could be simplified.
- if (svgSrc.includes('bus')) { - svgSrc = svgSrc.replace('fill="currentColor"', 'fill="white"'); - } else { - svgSrc = svgSrc.replace('fill="currentColor"', `fill="${color}"`); - } + const fillColor = svgSrc.includes('bus') ? 'white' : color; + svgSrc = svgSrc.replace('fill="currentColor"', `fill="${fillColor}"`);src/views/document/utils/boxes/IsReachableByPublicTransportsBox.vue (1)
42-50: Consider performance impact of deep watcherThe deep watcher on the document prop could cause performance issues if the document object is large and changes frequently.
Consider watching only the specific properties you need:
- watch: { - document: { - handler() { - this.checkAccessibility(); - }, - deep: true, - immediate: true, - }, - }, + watch: { + accessWaypoints: { + handler() { + this.checkAccessibility(); + }, + immediate: true, + }, + },src/components/map/OlMap.vue (1)
1067-1067: Remove unnecessary comment markersThe comment
///////////doesn't provide any value and should be removed.- this.setHighlightedFeature(resultFeature); /////////// + this.setHighlightedFeature(resultFeature);src/js/apis/transport-service.js (2)
5-10: Consider using ES6 class syntax for better readabilityThe service uses constructor function pattern. Consider modernizing to ES6 class syntax for improved readability and consistency with modern JavaScript practices.
-function TransportService() { - this.axios = axios.create({ - baseURL: config.urls.api, - }); -} +class TransportService { + constructor() { + this.axios = axios.create({ + baseURL: config.urls.api, + }); + } +}
17-29: Add input validation for waypointId parameterThe methods accept
waypointIdwithout validation. Consider adding checks to ensure the parameter is valid before making API calls.TransportService.prototype.getStopareas = function (waypointId) { + if (!waypointId) { + return Promise.reject(new Error('waypointId is required')); + } return this.axios.get(`/waypoints/${waypointId}/stopareas`); };src/views/document/utils/boxes/TransportsBox.vue (1)
495-497: Remove empty CSS class definitionsThere are empty class definitions that serve no purpose and should be removed.
- .missing-transports-warning-text { - } }- .public-transport-no-result-text { - } }Also applies to: 522-524
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
src/assets/img/boxes/public_transport.svgis excluded by!**/*.svgsrc/assets/img/boxes/toggle_minus.svgis excluded by!**/*.svgsrc/assets/img/boxes/toggle_plus.svgis excluded by!**/*.svgsrc/assets/img/boxes/transport_not_found.svgis excluded by!**/*.svg
📒 Files selected for processing (12)
.eslintrc.json(1 hunks)docker/nginx.conf(1 hunks)src/components/map/OlMap.vue(3 hunks)src/components/map/map-utils.js(4 hunks)src/js/apis/transport-service.js(1 hunks)src/translations/fr.json(16 hunks)src/views/document/RouteView.vue(2 hunks)src/views/document/WaypointView.vue(1 hunks)src/views/document/utils/boxes/IsReachableByPublicTransportsBox.vue(1 hunks)src/views/document/utils/boxes/TransportsBox.vue(1 hunks)src/views/document/utils/document-view-mixin.js(2 hunks)vue.config.js(3 hunks)
🔇 Additional comments (16)
docker/nginx.conf (1)
2-3: Verify the impact of removing proxy_protocol support.The change removes
proxy_protocolfrom the listen directive while keeping the original as a comment. This modification could affect how nginx handles connections from upstream proxies or load balancers that expect PROXY protocol support.Please confirm this change is intentional and won't break existing proxy configurations in production environments.
.eslintrc.json (1)
14-14: Good addition for cross-platform compatibility.The prettier/prettier rule with
"endOfLine": "auto"is a solid choice for handling line ending differences across different operating systems. This will help maintain consistent formatting while avoiding issues in mixed development environments.vue.config.js (2)
95-95: Verify the API endpoint change for demo environment.The demo environment API URL has been changed to the preprod URL. Please confirm this change is intentional and won't break demo environment functionality.
149-149: Default configuration changed from prod to localhost.The default configuration has been changed from production to localhost. This change could affect production builds if the BUILD_ENV is not properly set.
Please verify that production builds will set the appropriate BUILD_ENV to override this default.
src/views/document/WaypointView.vue (1)
132-132: Good integration of the transports-box component.The component is properly positioned and the document prop is correctly passed. The array wrapping (
[document]) suggests the TransportsBox component is designed to handle multiple documents, which is consistent with its usage pattern.src/views/document/utils/document-view-mixin.js (4)
5-5: Proper import of IsReachableByPublicTransportsBox component.The import is correctly placed in alphabetical order with other component imports.
10-10: Proper import of TransportsBox component.The import is correctly placed in alphabetical order with other component imports.
38-38: Good component registration for IsReachableByPublicTransportsBox.The component is properly registered in the components object, enabling its use in views that use this mixin.
46-46: Good component registration for TransportsBox.The component is properly registered in the components object, enabling its use in views that use this mixin.
src/translations/fr.json (1)
15-15: Translation entries look good!All new French translations for public transport features are properly formatted and grammatically correct.
Also applies to: 37-38, 78-78, 221-221, 310-310, 375-375, 394-394, 501-501, 530-530, 554-554, 594-594, 622-622, 643-643, 686-686, 710-710, 741-741
src/components/map/OlMap.vue (4)
1266-1277: LGTM!The
highlightStopmethod correctly resets styles and highlights the specified stop feature.
1279-1299: LGTM!The
goAndZoomOnStopmethod properly handles map animation with appropriate zoom level and animation cancellation.
1316-1333: LGTM!The
findStopPointmethod properly searches both document and waypoint layers with appropriate error handling.
1096-1102: Add defensive null checks for document accessThe code accesses
document.typewithout verifying thatdocumentexists. Whilefeature.get('document')is called at line 1095, it could return null/undefined.- if (document && document.type === 'z') { - return; - } - if (document && document.type === 's') { - this.$emit('stop-clicked', document.document_id); - return; - } + if (document) { + if (document.type === 'z') { + return; + } + if (document.type === 's') { + this.$emit('stop-clicked', document.document_id); + return; + } + }Likely an incorrect or invalid review comment.
src/js/apis/transport-service.js (1)
37-83: Well-implemented method with robust error handlingThe
getStopareasForDocumentsmethod handles edge cases well and efficiently fetches data in parallel. Good use of defensive programming with the null coalescing operator for the distance field.Consider adding a comment explaining the purpose of
documentResultsfor better code documentation.src/views/document/utils/boxes/TransportsBox.vue (1)
307-325: LGTM!The
seeLineDetailsmethod properly handles text selection check and smooth scrolling behavior.
| prod: { | ||
| name: 'prod', | ||
| api: 'https://api.camptocamp.org', | ||
| api: 'https://preprod.c2c.preprod0.smart-origin.com/api', |
There was a problem hiding this comment.
Both demo and prod environments now use the same API endpoint.
Both demo and prod configurations now point to the same preprod URL (https://preprod.c2c.preprod0.smart-origin.com/api). This seems incorrect as they should typically point to different environments.
🤖 Prompt for AI Agents
In vue.config.js at line 117, the API endpoint for both demo and prod
environments is set to the same preprod URL, which is incorrect. Update the prod
environment configuration to use the correct production API endpoint URL instead
of the preprod URL, ensuring demo and prod environments point to their
respective distinct API endpoints.
| localhost: { | ||
| name: 'localhost', | ||
| api: 'http://localhost:6543', | ||
| media: 'https://sos-ch-dk-2.exo.io/c2corg-demov6-active', | ||
| imageBackend: 'https://images.demov6.camptocamp.org', | ||
| forum: 'https://forum.demov6.camptocamp.org', | ||
| modernThumbnailsTimestamp: 0, | ||
| modernThumbnailsId: 0, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate localhost configuration detected.
The localhost configuration is defined here and again in the conditional block starting at line 158. This duplication could lead to confusion and maintenance issues.
Consider removing this duplicate configuration and relying on the conditional block below.
🤖 Prompt for AI Agents
In vue.config.js between lines 137 and 145, there is a duplicate localhost
configuration also defined in the conditional block starting at line 158. To fix
this, remove the localhost configuration from lines 137 to 145 and keep only the
one in the conditional block to avoid confusion and simplify maintenance.
| <div class="column is-3 no-print"> | ||
| <map-box :document="document" @has-protection-area="hasProtectionArea = true" /> | ||
| <avalanche-box :document="document" v-if="!$screen.isMobile" /> | ||
| <is-reachable-by-public-transports-box :document="document.associations.waypoints" /> |
There was a problem hiding this comment.
Add null safety check for waypoints prop
The component accesses nested properties without checking if they exist. This could cause runtime errors if associations or waypoints are undefined.
Consider adding a computed property or default value:
-<is-reachable-by-public-transports-box :document="document.associations.waypoints" />
+<is-reachable-by-public-transports-box :document="document.associations?.waypoints || []" />Or handle this in a computed property:
computed: {
associatedWaypoints() {
return this.document?.associations?.waypoints || [];
}
}🤖 Prompt for AI Agents
In src/views/document/RouteView.vue at line 14, the code passes
document.associations.waypoints directly without checking if associations or
waypoints exist, risking runtime errors. Fix this by adding a computed property
that safely returns document.associations.waypoints or an empty array if
undefined, and then bind this computed property to the
is-reachable-by-public-transports-box component instead of accessing nested
properties directly.
| <div style="clear: both" /> | ||
| </div> | ||
|
|
||
| <transports-box :document="document.associations.waypoints" /> |
There was a problem hiding this comment.
Add null safety check for waypoints prop
Same issue as above - accessing nested properties without null checks.
Apply the same defensive programming approach:
-<transports-box :document="document.associations.waypoints" />
+<transports-box :document="document.associations?.waypoints || []" />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <transports-box :document="document.associations.waypoints" /> | |
| <transports-box :document="document.associations?.waypoints || []" /> |
🤖 Prompt for AI Agents
In src/views/document/RouteView.vue at line 128, the code accesses
document.associations.waypoints without checking if document or associations is
null or undefined. Add a null safety check to ensure document and
document.associations exist before accessing waypoints, for example by using
optional chaining or a conditional check, to prevent runtime errors.
| } else if (type === 'z') { | ||
| svgSrc = svgSrcByDocumentType['z'] || icon({ prefix: 'waypoint', iconName: 'access' }).html[0]; | ||
|
|
||
| if (document.public_transportation_rating && document.public_transportation_rating !== 'no service') { | ||
| color = '#4baf50'; | ||
| } else { | ||
| color = '#F93'; | ||
| } | ||
|
|
||
| const styles = buildPointStyle(title, svgSrc, color, highlight); | ||
|
|
||
| styles.forEach((style) => { | ||
| const image = style.getImage(); | ||
|
|
||
| if (image instanceof ol.style.Circle) { | ||
| image.getFill().setColor('rgba(255, 255, 255, 1)'); | ||
| image.getStroke().setColor('#4baf50'); | ||
|
|
||
| style.setZIndex(highlight ? 101 : 50); | ||
| } | ||
|
|
||
| if (image instanceof ol.style.Icon) { | ||
| style.setZIndex(highlight ? 102 : 51); | ||
| } | ||
| }); | ||
|
|
||
| return styles; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract magic numbers as constants and optimize style iteration
The code uses magic numbers for z-index values and iterates through styles inefficiently.
Add constants at the top of the file:
const Z_INDEX = {
NORMAL: { CIRCLE: 1, ICON: 2 },
HIGHLIGHT: { CIRCLE: 101, ICON: 102 },
ACCESS_NORMAL: { CIRCLE: 50, ICON: 51 },
ACCESS_HIGHLIGHT: { CIRCLE: 101, ICON: 102 }
};Then refactor the type 'z' logic:
- const styles = buildPointStyle(title, svgSrc, color, highlight);
-
- styles.forEach((style) => {
- const image = style.getImage();
-
- if (image instanceof ol.style.Circle) {
- image.getFill().setColor('rgba(255, 255, 255, 1)');
- image.getStroke().setColor('#4baf50');
-
- style.setZIndex(highlight ? 101 : 50);
- }
-
- if (image instanceof ol.style.Icon) {
- style.setZIndex(highlight ? 102 : 51);
- }
- });
-
- return styles;
+ const styles = buildPointStyle(title, svgSrc, color, highlight);
+ const [circleStyle, iconStyle] = styles;
+
+ // Customize circle style
+ const circleImage = circleStyle.getImage();
+ circleImage.getFill().setColor('rgba(255, 255, 255, 1)');
+ circleImage.getStroke().setColor('#4baf50');
+ circleStyle.setZIndex(highlight ? Z_INDEX.ACCESS_HIGHLIGHT.CIRCLE : Z_INDEX.ACCESS_NORMAL.CIRCLE);
+
+ // Customize icon style
+ iconStyle.setZIndex(highlight ? Z_INDEX.ACCESS_HIGHLIGHT.ICON : Z_INDEX.ACCESS_NORMAL.ICON);
+
+ return styles;🤖 Prompt for AI Agents
In src/components/map/map-utils.js between lines 139 and 165, replace all
hardcoded z-index numbers with named constants for clarity and maintainability.
Define a Z_INDEX constant object at the top of the file with properties for
normal and highlight states for both circle and icon styles as suggested. Then
update the style iteration logic to use these constants instead of magic numbers
when calling style.setZIndex, ensuring the correct z-index is applied based on
whether the style is highlighted and the type of image (Circle or Icon).
| console.error( | ||
| `Erreur lors de la vérification de l'accessibilité pour le waypoint ${waypointIds[index]} :`, | ||
| error | ||
| ); |
There was a problem hiding this comment.
Use i18n for error messages
Error message is hardcoded in French, which breaks internationalization.
- console.error(
- `Erreur lors de la vérification de l'accessibilité pour le waypoint ${waypointIds[index]} :`,
- error
- );
+ console.error(
+ this.$gettext('Error checking accessibility for waypoint %{id}:', { id: waypointIds[index] }),
+ error
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.error( | |
| `Erreur lors de la vérification de l'accessibilité pour le waypoint ${waypointIds[index]} :`, | |
| error | |
| ); | |
| console.error( | |
| this.$gettext('Error checking accessibility for waypoint %{id}:', { id: waypointIds[index] }), | |
| error | |
| ); |
🤖 Prompt for AI Agents
In src/views/document/utils/boxes/IsReachableByPublicTransportsBox.vue around
lines 94 to 97, the error message is hardcoded in French, which breaks
internationalization. Replace the hardcoded French error string with a call to
the i18n translation function, passing the error message key and any dynamic
parameters like waypointIds[index] to ensure the message is localized properly.
| resetStopStyles() { | ||
| const documentsSource = this.documentsLayer.getSource(); | ||
| const waypointsSource = this.waypointsLayer.getSource(); | ||
|
|
||
| documentsSource.forEachFeature((feature) => { | ||
| if (feature.values_.document.type !== 'w' && feature.values_.document.type !== 'z') { | ||
| feature.setStyle(feature.get('normalStyle')); | ||
| } | ||
| }); | ||
|
|
||
| waypointsSource.forEachFeature((feature) => { | ||
| feature.setStyle(feature.get('normalStyle')); | ||
| }); | ||
| }, |
There was a problem hiding this comment.
Avoid accessing internal properties of OpenLayers features
The code accesses feature.values_.document.type which uses OpenLayers' internal property structure. This is fragile and could break with library updates.
documentsSource.forEachFeature((feature) => {
- if (feature.values_.document.type !== 'w' && feature.values_.document.type !== 'z') {
+ const document = feature.get('document');
+ if (document && document.type !== 'w' && document.type !== 'z') {
feature.setStyle(feature.get('normalStyle'));
}
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| resetStopStyles() { | |
| const documentsSource = this.documentsLayer.getSource(); | |
| const waypointsSource = this.waypointsLayer.getSource(); | |
| documentsSource.forEachFeature((feature) => { | |
| if (feature.values_.document.type !== 'w' && feature.values_.document.type !== 'z') { | |
| feature.setStyle(feature.get('normalStyle')); | |
| } | |
| }); | |
| waypointsSource.forEachFeature((feature) => { | |
| feature.setStyle(feature.get('normalStyle')); | |
| }); | |
| }, | |
| resetStopStyles() { | |
| const documentsSource = this.documentsLayer.getSource(); | |
| const waypointsSource = this.waypointsLayer.getSource(); | |
| documentsSource.forEachFeature((feature) => { | |
| + const document = feature.get('document'); | |
| + if (document && document.type !== 'w' && document.type !== 'z') { | |
| feature.setStyle(feature.get('normalStyle')); | |
| + } | |
| }); | |
| waypointsSource.forEachFeature((feature) => { | |
| feature.setStyle(feature.get('normalStyle')); | |
| }); | |
| }, |
🤖 Prompt for AI Agents
In src/components/map/OlMap.vue around lines 1301 to 1314, the code accesses the
internal property feature.values_.document.type, which is fragile and may break
with OpenLayers updates. To fix this, replace the direct access to values_ with
the official OpenLayers API method feature.get('document') to retrieve the
document object, then check its type property safely. This avoids relying on
internal properties and ensures compatibility with future OpenLayers versions.
| mixins: [requireDocumentProperty], | ||
| props: { | ||
| document: Array, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clarify the document prop type and consider renaming
The document prop is typed as Array which is unusual for a property named "document". This could be confusing for developers. Consider either:
- Renaming the prop to better reflect its array nature (e.g.,
documentsorwaypoints) - Adding validation or documentation to clarify what this array contains
props: {
- document: Array,
+ waypoints: {
+ type: Array,
+ default: () => [],
+ validator: (value) => value.every(item => item && typeof item === 'object')
+ },
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mixins: [requireDocumentProperty], | |
| props: { | |
| document: Array, | |
| }, | |
| mixins: [requireDocumentProperty], | |
| props: { | |
| waypoints: { | |
| type: Array, | |
| default: () => [], | |
| validator: (value) => value.every(item => item && typeof item === 'object') | |
| }, | |
| }, |
🤖 Prompt for AI Agents
In src/views/document/utils/boxes/TransportsBox.vue around lines 96 to 99, the
prop named "document" is typed as an Array, which is misleading given the name.
To fix this, either rename the prop to a plural form like "documents" or
"waypoints" to reflect that it is an array, or keep the name but add detailed
validation or documentation specifying the expected contents and structure of
the array to clarify its purpose.
| fetchStopsForDocuments(documents) { | ||
| transportService | ||
| .getStopareasForDocuments(documents) | ||
| .then((result) => { | ||
| documents.forEach((doc) => { | ||
| if (!result.documentResults[doc.document_id]) { | ||
| doc.public_transportation_rating = 'no service'; | ||
| } else { | ||
| doc.public_transportation_rating = 'service available'; | ||
| } | ||
| }); | ||
|
|
||
| this.stops = result.stopareas.sort((a, b) => a.distance - b.distance); | ||
| this.missingTransportForWaypoint = result.missingTransportForWaypoint; | ||
| this.showAccessibilityInfo = this.stops.length > 0; | ||
| this.createStopDocuments(); | ||
| }) | ||
| .catch((error) => { | ||
| console.error('Erreur lors de la récupération des données :', error); | ||
| this.missingTransportForWaypoint = true; | ||
|
|
||
| documents.forEach((doc) => { | ||
| doc.public_transportation_rating = 'no service'; | ||
| }); | ||
| }); | ||
| }, |
There was a problem hiding this comment.
Avoid mutating input documents directly
The method modifies the input documents array by setting public_transportation_rating directly. This mutation can cause unexpected side effects and makes the component harder to test and reason about.
Consider creating a local copy or using a computed property to track transportation ratings:
fetchStopsForDocuments(documents) {
+ const documentRatings = {};
transportService
.getStopareasForDocuments(documents)
.then((result) => {
documents.forEach((doc) => {
if (!result.documentResults[doc.document_id]) {
- doc.public_transportation_rating = 'no service';
+ documentRatings[doc.document_id] = 'no service';
} else {
- doc.public_transportation_rating = 'service available';
+ documentRatings[doc.document_id] = 'service available';
}
});
+ this.documentRatings = documentRatings;Also, use English for console messages:
- console.error('Erreur lors de la récupération des données :', error);
+ console.error('Error fetching transport data:', error);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fetchStopsForDocuments(documents) { | |
| transportService | |
| .getStopareasForDocuments(documents) | |
| .then((result) => { | |
| documents.forEach((doc) => { | |
| if (!result.documentResults[doc.document_id]) { | |
| doc.public_transportation_rating = 'no service'; | |
| } else { | |
| doc.public_transportation_rating = 'service available'; | |
| } | |
| }); | |
| this.stops = result.stopareas.sort((a, b) => a.distance - b.distance); | |
| this.missingTransportForWaypoint = result.missingTransportForWaypoint; | |
| this.showAccessibilityInfo = this.stops.length > 0; | |
| this.createStopDocuments(); | |
| }) | |
| .catch((error) => { | |
| console.error('Erreur lors de la récupération des données :', error); | |
| this.missingTransportForWaypoint = true; | |
| documents.forEach((doc) => { | |
| doc.public_transportation_rating = 'no service'; | |
| }); | |
| }); | |
| }, | |
| fetchStopsForDocuments(documents) { | |
| const documentRatings = {}; | |
| transportService | |
| .getStopareasForDocuments(documents) | |
| .then((result) => { | |
| documents.forEach((doc) => { | |
| if (!result.documentResults[doc.document_id]) { | |
| documentRatings[doc.document_id] = 'no service'; | |
| } else { | |
| documentRatings[doc.document_id] = 'service available'; | |
| } | |
| }); | |
| this.documentRatings = documentRatings; | |
| this.stops = result.stopareas.sort((a, b) => a.distance - b.distance); | |
| this.missingTransportForWaypoint = result.missingTransportForWaypoint; | |
| this.showAccessibilityInfo = this.stops.length > 0; | |
| this.createStopDocuments(); | |
| }) | |
| .catch((error) => { | |
| console.error('Error fetching transport data:', error); | |
| this.missingTransportForWaypoint = true; | |
| documents.forEach((doc) => { | |
| doc.public_transportation_rating = 'no service'; | |
| }); | |
| }); | |
| }, |
🤖 Prompt for AI Agents
In src/views/document/utils/boxes/TransportsBox.vue around lines 194 to 219, the
fetchStopsForDocuments method directly mutates the input documents array by
setting public_transportation_rating on each document, which can cause side
effects. To fix this, create a local copy of the documents or maintain a
separate data structure to track transportation ratings without modifying the
original input. Also, replace the French console error message with an English
one for consistency.
| createStopDocuments() { | ||
| if (!this.stops || !this.stops.length) return; | ||
|
|
||
| if (this.accessWaypoints.length === 0) return; | ||
|
|
||
| this.stopDocuments = this.stops.reduce( | ||
| (uniqueStops, stop) => { | ||
| try { | ||
| if (!stop.coordinates || !stop.coordinates.x || !stop.coordinates.y) { | ||
| console.warn(`Stop ${stop.id} n'a pas de coordonnées valides:`, stop); | ||
| return uniqueStops; | ||
| } | ||
|
|
||
| const coordKey = `${stop.coordinates.x},${stop.coordinates.y}`; | ||
| if (uniqueStops.coordsMap[coordKey]) { | ||
| return uniqueStops; | ||
| } | ||
|
|
||
| const stopDoc = JSON.parse(JSON.stringify(this.accessWaypoints[0])); | ||
| stopDoc.document_id = stop.id; | ||
| stopDoc.type = 's'; | ||
| if (stopDoc.locales && stopDoc.locales.length > 0) { | ||
| stopDoc.locales[0].title = ''; | ||
| } | ||
| if (stopDoc.geometry) { | ||
| const geom = { | ||
| type: 'Point', | ||
| coordinates: [stop.coordinates.x, stop.coordinates.y], | ||
| }; | ||
| stopDoc.geometry.geom = JSON.stringify(geom); | ||
| } | ||
| stopDoc.isStopPoint = true; | ||
| stopDoc.stopInfo = { | ||
| operator: stop.operator, | ||
| line: stop.line, | ||
| distance: stop.distance, | ||
| }; | ||
|
|
||
| uniqueStops.coordsMap[coordKey] = true; | ||
| uniqueStops.docs.push(stopDoc); | ||
| return uniqueStops; | ||
| } catch (error) { | ||
| console.error(`Erreur lors de la création du document pour le stop ${stop.id}:`, error); | ||
| return uniqueStops; | ||
| } | ||
| }, | ||
| { docs: [], coordsMap: {} } | ||
| ).docs; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve deep cloning approach and use English for logs
The method uses JSON.parse(JSON.stringify()) for deep cloning which is inefficient and can fail with certain data types (dates, functions, undefined values).
Consider using a proper deep clone utility:
- const stopDoc = JSON.parse(JSON.stringify(this.accessWaypoints[0]));
+ const stopDoc = this.$utils.deepClone(this.accessWaypoints[0]);Or use object spread with nested properties:
- const stopDoc = JSON.parse(JSON.stringify(this.accessWaypoints[0]));
+ const stopDoc = {
+ ...this.accessWaypoints[0],
+ geometry: this.accessWaypoints[0].geometry ? { ...this.accessWaypoints[0].geometry } : null,
+ locales: this.accessWaypoints[0].locales?.map(locale => ({ ...locale })) || [],
+ };Also, use English for console messages:
- console.warn(`Stop ${stop.id} n'a pas de coordonnées valides:`, stop);
+ console.warn(`Stop ${stop.id} has no valid coordinates:`, stop);- console.error(`Erreur lors de la création du document pour le stop ${stop.id}:`, error);
+ console.error(`Error creating document for stop ${stop.id}:`, error);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| createStopDocuments() { | |
| if (!this.stops || !this.stops.length) return; | |
| if (this.accessWaypoints.length === 0) return; | |
| this.stopDocuments = this.stops.reduce( | |
| (uniqueStops, stop) => { | |
| try { | |
| if (!stop.coordinates || !stop.coordinates.x || !stop.coordinates.y) { | |
| console.warn(`Stop ${stop.id} n'a pas de coordonnées valides:`, stop); | |
| return uniqueStops; | |
| } | |
| const coordKey = `${stop.coordinates.x},${stop.coordinates.y}`; | |
| if (uniqueStops.coordsMap[coordKey]) { | |
| return uniqueStops; | |
| } | |
| const stopDoc = JSON.parse(JSON.stringify(this.accessWaypoints[0])); | |
| stopDoc.document_id = stop.id; | |
| stopDoc.type = 's'; | |
| if (stopDoc.locales && stopDoc.locales.length > 0) { | |
| stopDoc.locales[0].title = ''; | |
| } | |
| if (stopDoc.geometry) { | |
| const geom = { | |
| type: 'Point', | |
| coordinates: [stop.coordinates.x, stop.coordinates.y], | |
| }; | |
| stopDoc.geometry.geom = JSON.stringify(geom); | |
| } | |
| stopDoc.isStopPoint = true; | |
| stopDoc.stopInfo = { | |
| operator: stop.operator, | |
| line: stop.line, | |
| distance: stop.distance, | |
| }; | |
| uniqueStops.coordsMap[coordKey] = true; | |
| uniqueStops.docs.push(stopDoc); | |
| return uniqueStops; | |
| } catch (error) { | |
| console.error(`Erreur lors de la création du document pour le stop ${stop.id}:`, error); | |
| return uniqueStops; | |
| } | |
| }, | |
| { docs: [], coordsMap: {} } | |
| ).docs; | |
| }, | |
| createStopDocuments() { | |
| if (!this.stops || !this.stops.length) return; | |
| if (this.accessWaypoints.length === 0) return; | |
| this.stopDocuments = this.stops.reduce( | |
| (uniqueStops, stop) => { | |
| try { | |
| if (!stop.coordinates || !stop.coordinates.x || !stop.coordinates.y) { | |
| console.warn(`Stop ${stop.id} has no valid coordinates:`, stop); | |
| return uniqueStops; | |
| } | |
| const coordKey = `${stop.coordinates.x},${stop.coordinates.y}`; | |
| if (uniqueStops.coordsMap[coordKey]) { | |
| return uniqueStops; | |
| } | |
| const stopDoc = this.$utils.deepClone(this.accessWaypoints[0]); | |
| stopDoc.document_id = stop.id; | |
| stopDoc.type = 's'; | |
| if (stopDoc.locales && stopDoc.locales.length > 0) { | |
| stopDoc.locales[0].title = ''; | |
| } | |
| if (stopDoc.geometry) { | |
| const geom = { | |
| type: 'Point', | |
| coordinates: [stop.coordinates.x, stop.coordinates.y], | |
| }; | |
| stopDoc.geometry.geom = JSON.stringify(geom); | |
| } | |
| stopDoc.isStopPoint = true; | |
| stopDoc.stopInfo = { | |
| operator: stop.operator, | |
| line: stop.line, | |
| distance: stop.distance, | |
| }; | |
| uniqueStops.coordsMap[coordKey] = true; | |
| uniqueStops.docs.push(stopDoc); | |
| return uniqueStops; | |
| } catch (error) { | |
| console.error(`Error creating document for stop ${stop.id}:`, error); | |
| return uniqueStops; | |
| } | |
| }, | |
| { docs: [], coordsMap: {} } | |
| ).docs; | |
| }, |
🤖 Prompt for AI Agents
In src/views/document/utils/boxes/TransportsBox.vue between lines 223 and 271,
replace the JSON.parse(JSON.stringify()) deep cloning with a more reliable deep
clone utility or a safer cloning method like object spread for nested properties
to handle complex data types correctly. Additionally, update all console warning
and error messages to be in English for consistency and clarity.
3 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores