From: Tom Hughes Date: Wed, 12 Feb 2025 17:59:25 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/5622' X-Git-Tag: live~61 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/22f590c9ef38ce3a1745be4cc3f411c763696c07?hp=4133936c6304b7d1a57c85d53fb7c3a906190164 Merge remote-tracking branch 'upstream/pull/5622' --- diff --git a/app/assets/javascripts/index/directions.js b/app/assets/javascripts/index/directions.js index 356dc6271..0ac66d9d6 100644 --- a/app/assets/javascripts/index/directions.js +++ b/app/assets/javascripts/index/directions.js @@ -3,7 +3,7 @@ //= require_tree ./directions OSM.Directions = function (map) { - var routeRequest = null; // jqXHR object of an ongoing route request or null + let controller = null; // the AbortController for the current route request if a route request is in progress var chosenEngine; var popup = L.popup({ autoPanPadding: [100, 100] }); @@ -23,7 +23,7 @@ OSM.Directions = function (map) { var endpointDragCallback = function (dragging) { if (!map.hasLayer(polyline)) return; if (dragging && !chosenEngine.draggable) return; - if (dragging && routeRequest) return; + if (dragging && controller) return; getRoute(false, !dragging); }; @@ -109,7 +109,7 @@ OSM.Directions = function (map) { function getRoute(fitRoute, reportErrors) { // Cancel any route that is already in progress - if (routeRequest) routeRequest.abort(); + if (controller) controller.abort(); const points = endpoints.map(p => p.latlng); @@ -126,20 +126,8 @@ OSM.Directions = function (map) { // again. $("#sidebar_content").html($(".directions_form .loader_copy").html()); map.setSidebarOverlaid(false); - - routeRequest = chosenEngine.getRoute(points, function (err, route) { - routeRequest = null; - - if (err) { - map.removeLayer(polyline); - - if (reportErrors) { - $("#sidebar_content").html("
" + I18n.t("javascripts.directions.errors.no_route") + "
"); - } - - return; - } - + controller = new AbortController(); + chosenEngine.getRoute(points, controller.signal).then(function (route) { polyline .setLatLngs(route.line) .addTo(map); @@ -212,6 +200,13 @@ OSM.Directions = function (map) { map.setSidebarOverlaid(true); // TODO: collapse width of sidebar back to previous }); + }).catch(function () { + map.removeLayer(polyline); + if (reportErrors) { + $("#sidebar_content").html("
" + I18n.t("javascripts.directions.errors.no_route") + "
"); + } + }).finally(function () { + controller = null; }); function getDistText(dist) { diff --git a/app/assets/javascripts/index/directions/fossgis_osrm.js b/app/assets/javascripts/index/directions/fossgis_osrm.js index bb968f2da..cd1731247 100644 --- a/app/assets/javascripts/index/directions/fossgis_osrm.js +++ b/app/assets/javascripts/index/directions/fossgis_osrm.js @@ -154,15 +154,15 @@ creditline: "OSRM (FOSSGIS)", draggable: true, - getRoute: function (points, callback) { - const data = [ - { name: "overview", value: "false" }, - { name: "geometries", value: "polyline" }, - { name: "steps", value: true } - ]; + getRoute: function (points, signal) { + const query = new URLSearchParams({ + overview: "false", + geometries: "polyline", + steps: true + }); if (cachedHints.length === points.length) { - data.push({ name: "hints", value: cachedHints.join(";") }); + query.set("hints", cachedHints.join(";")); } else { // invalidate cache cachedHints = []; @@ -170,22 +170,13 @@ const req_path = "routed-" + vehicleType + "/route/v1/driving/" + points.map(p => p.lng + "," + p.lat).join(";"); - return $.ajax({ - url: OSM.FOSSGIS_OSRM_URL + req_path, - data, - dataType: "json", - success: function (response) { - if (response.code !== "Ok") { - return callback(true); - } - + return fetch(OSM.FOSSGIS_OSRM_URL + req_path + "?" + query, { signal }) + .then(response => response.json()) + .then(response => { + if (response.code !== "Ok") throw new Error(); cachedHints = response.waypoints.map(wp => wp.hint); - callback(false, _processDirections(response.routes[0])); - }, - error: function () { - callback(true); - } - }); + return _processDirections(response.routes[0]); + }); } }; } diff --git a/app/assets/javascripts/index/directions/fossgis_valhalla.js b/app/assets/javascripts/index/directions/fossgis_valhalla.js index bbccccb13..41ad6a972 100644 --- a/app/assets/javascripts/index/directions/fossgis_valhalla.js +++ b/app/assets/javascripts/index/directions/fossgis_valhalla.js @@ -87,8 +87,8 @@ "Valhalla (FOSSGIS)", draggable: false, - getRoute: function (points, callback) { - const data = { + getRoute: function (points, signal) { + const query = new URLSearchParams({ json: JSON.stringify({ locations: points.map(function (p) { return { lat: p.lat, lon: p.lng, radius: 5 }; @@ -99,22 +99,13 @@ language: I18n.currentLocale() } }) - }; - return $.ajax({ - url: OSM.FOSSGIS_VALHALLA_URL, - data, - dataType: "json", - success: function ({ trip }) { - if (trip.status === 0) { - callback(false, _processDirections(trip.legs)); - } else { - callback(true); - } - }, - error: function () { - callback(true); - } }); + return fetch(OSM.FOSSGIS_VALHALLA_URL + "?" + query, { signal }) + .then(response => response.json()) + .then(({ trip }) => { + if (trip.status !== 0) throw new Error(); + return _processDirections(trip.legs); + }); } }; } diff --git a/app/assets/javascripts/index/directions/graphhopper.js b/app/assets/javascripts/index/directions/graphhopper.js index 191475873..729618f2d 100644 --- a/app/assets/javascripts/index/directions/graphhopper.js +++ b/app/assets/javascripts/index/directions/graphhopper.js @@ -51,33 +51,24 @@ creditline: "GraphHopper", draggable: false, - getRoute: function (points, callback) { + getRoute: function (points, signal) { // GraphHopper Directions API documentation // https://graphhopper.com/api/1/docs/routing/ - const data = { + const query = new URLSearchParams({ vehicle: vehicleType, locale: I18n.currentLocale(), key: "LijBPDQGfu7Iiq80w3HzwB4RUDJbMbhs6BU0dEnn", elevation: false, instructions: true, - turn_costs: vehicleType === "car", - point: points.map(p => p.lat + "," + p.lng) - }; - return $.ajax({ - url: OSM.GRAPHHOPPER_URL, - data, - traditional: true, - dataType: "json", - success: function ({ paths }) { - if (!paths || paths.length === 0) { - return callback(true); - } - callback(false, _processDirections(paths[0])); - }, - error: function () { - callback(true); - } + turn_costs: vehicleType === "car" }); + points.forEach(p => query.append("point", p.lat + "," + p.lng)); + return fetch(OSM.GRAPHHOPPER_URL + "?" + query, { signal }) + .then(response => response.json()) + .then(({ paths }) => { + if (!paths || paths.length === 0) throw new Error(); + return _processDirections(paths[0]); + }); } }; } diff --git a/app/controllers/api/notes_controller.rb b/app/controllers/api/notes_controller.rb index f6d6dede9..bc4d2eaf2 100644 --- a/app/controllers/api/notes_controller.rb +++ b/app/controllers/api/notes_controller.rb @@ -81,19 +81,22 @@ module Api # Extract the arguments lon = OSM.parse_float(params[:lon], OSM::APIBadUserInput, "lon was not a number") lat = OSM.parse_float(params[:lat], OSM::APIBadUserInput, "lat was not a number") - comment = params[:text] + description = params[:text] + + # Get note's author info (for logged in users - user_id, for logged out users - IP address) + note_author_info = author_info # Include in a transaction to ensure that there is always a note_comment for every note Note.transaction do # Create the note - @note = Note.create(:lat => lat, :lon => lon) + @note = Note.create(:lat => lat, :lon => lon, :description => description, :user_id => note_author_info[:user_id], :user_ip => note_author_info[:user_ip]) raise OSM::APIBadUserInput, "The note is outside this world" unless @note.in_world? # Save the note @note.save! - # Add a comment to the note - add_comment(@note, comment, "opened") + # Add opening comment (description) to the note + add_comment(@note, description, "opened") end # Return a copy of the new note @@ -381,17 +384,28 @@ module Api end end + ## + # Get author's information (for logged in users - user_id, for logged out users - IP address) + def author_info + if scope_enabled?(:write_notes) + { :user_id => current_user.id } + else + { :user_ip => request.remote_ip } + end + end + ## # Add a comment to a note def add_comment(note, text, event, notify: true) attributes = { :visible => true, :event => event, :body => text } - author = current_user if scope_enabled?(:write_notes) + # Get note comment's author info (for logged in users - user_id, for logged out users - IP address) + note_comment_author_info = author_info - if author - attributes[:author_id] = author.id + if note_comment_author_info[:user_ip].nil? + attributes[:author_id] = note_comment_author_info[:user_id] else - attributes[:author_ip] = request.remote_ip + attributes[:author_ip] = note_comment_author_info[:user_ip] end comment = note.comments.create!(attributes) diff --git a/test/system/directions_test.rb b/test/system/directions_test.rb index 08f5cb8e3..91a033c4d 100644 --- a/test/system/directions_test.rb +++ b/test/system/directions_test.rb @@ -37,7 +37,7 @@ class DirectionsSystemTest < ApplicationSystemTestCase stub_routing <<~CALLBACK const distance = points[0].distanceTo(points[1]); const time = distance * 30; - callback(false, { + return Promise.resolve({ line: points, steps: [ [points[0], 8, "1. #{start_instruction}", distance, points], @@ -53,10 +53,8 @@ class DirectionsSystemTest < ApplicationSystemTestCase execute_script <<~SCRIPT $(() => { for (const engine of OSM.Directions.engines) { - engine.getRoute = (points, callback) => { - setTimeout(() => { + engine.getRoute = (points, signal) => { #{callback_code} - }); }; } });