search

pelias / api

HTTP API for Pelias Geocoder
http://pelias.io
MIT License
218 stars 162 forks source link

update pelias/sorting module to fix numeric sorting bug #1626

Closed missinglink closed 2 years ago

missinglink commented 2 years ago

this PR fixes a subtle sorting bug: https://github.com/pelias/sorting/pull/26

in particular, a user noted that the query Acton, ME, USA returned the result for Meeker County above that of Maine, which is now resolved.

missinglink commented 2 years ago

I've added a second commit which removes the stable module since stable sort has been available natively since node@12.

Note: the comparator is expected to return a finite number, I hunted for any cases where it might return a boolean and couldn't find any in the code, one in the tests, worth keeping an 👁️ out for.

missinglink commented 2 years ago

acceptance tests look good, there was one regression:

/v1/search?sources=wof&text=Taman Taynton View, Malaysia

# previously
0) Taman Taynton View, Kuala Lumpur, Malaysia
1) Taman Taynton, Kuala Lumpur, Malaysia

# now
0) Taman Taynton, Kuala Lumpur, Malaysia
1) Taman Taynton View, Kuala Lumpur, Malaysia

This is due to the results being displayed in order returned from pelias/placeholder.

I'm happy to merge this as-is, we can tackle the sorting order returned by placeholder in a subsequent PR.