[openstreetmap/openstreetmap-website] Transition more requests to fetch (PR #5669)

Tom Hughes notifications at github.com
Thu Feb 13 19:17:15 UTC 2025


@tomhughes commented on this pull request.



>            $("#browse_status").empty();
-        }
-
-        if (XMLHttpRequest.status === 400 && XMLHttpRequest.responseText) {
-          displayLoadError(XMLHttpRequest.responseText, closeError);
-        } else if (XMLHttpRequest.statusText) {
-          displayLoadError(XMLHttpRequest.statusText, closeError);
-        } else {
-          displayLoadError(String(XMLHttpRequest.status), closeError);
-        }
-      }
-    });
+        });
+      }).finally(() => dataLoader = null);

Can we put the `.finally` on a new line for consistency with other places?

> @@ -71,30 +71,28 @@ OSM.initializeNotesLayer = function (map) {
     var size = bounds.getSize();
 
     if (size <= OSM.MAX_NOTE_REQUEST_AREA) {
-      var url = "/api/" + OSM.API_VERSION + "/notes.json?bbox=" + bounds.toBBoxString();
+      var url = "/api/" + OSM.API_VERSION + "/notes?bbox=" + bounds.toBBoxString();

Was there a reason for preferring sending an accept header over using the `.json` extension?

>  
-      noteLoader = null;
+          for (var id in oldNotes) {
+            noteLayer.removeLayer(oldNotes[id]);
+          }
+        })
+        .catch(() => {})

Do we actually need this `.catch` if it doesn't do anything?

>        method: "POST",
-      data: {
+      headers: { "Content-Type": "application/x-www-form-urlencoded" },

Is this necessary? I didn't actually check what it was sending when I did my version but it seemed to work and I assumed having `URLSearchParams` as the body was enough of a hint to make it send that type?

>  
     $(this).hide();
     div.find(".loader").show();
 
-    params[csrf_param] = csrf_token;
+    params.append(csrf_param, csrf_token);

Was there a reason for using `.append` here rather than `.set` like other places?


-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5669#pullrequestreview-2615987025
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/5669/review/2615987025 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20250213/73ed0543/attachment.htm>


More information about the rails-dev mailing list