-
Notifications
You must be signed in to change notification settings - Fork 66
Add _insidePolygon support to geosearch #1414
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: a305a0d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds insidePolygon support across the adapter, tests, docs, and playgrounds: adapters accept and validate polygon coordinates (≥3 points), produce a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as InstantSearch Widget
participant SPA as search-params-adapter
participant GRA as geo-rules-adapter
participant MS as Meilisearch
UI->>SPA: build search params (insidePolygon, insideBoundingBox, around*)
SPA->>GRA: adaptGeoSearch({ insidePolygon, ... })
alt insidePolygon valid (>=3 points)
note right of GRA #DFF2E1: Validate points, drop NaN\nFormat into string pairs\nRequire docs to have `_geojson`
GRA-->>SPA: return `_geoPolygon` filter
else invalid or absent polygon
note right of GRA #FFF4E5: Fallback to aroundLatLng/aroundRadius\nor insideBoundingBox (existing logic)
GRA-->>SPA: return other geo filter or undefined
end
SPA->>MS: search(query, filters including _geoPolygon if present)
MS-->>SPA: hits
SPA-->>UI: results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/instant-meilisearch/src/adapter/search-request-adapter/geo-rules-adapter.ts (1)
19-22: Consider adding precision control for consistency.Unlike the existing
aroundLatLnghandling (line 35) which uses.toFixed(5)to control floating-point precision, the polygon coordinates don't limit decimal places. While not critical, this inconsistency could lead to unexpected filter strings with high-precision input.Apply this diff for consistency:
- const lat = Number.parseFloat(String(pair[0])) - const lng = Number.parseFloat(String(pair[1])) + const lat = Number.parseFloat(String(pair[0])).toFixed(5) + const lng = Number.parseFloat(String(pair[1])).toFixed(5) if (Number.isNaN(lat) || Number.isNaN(lng)) return null - return `[${lat}, ${lng}]` + return `[${Number(lat)}, ${Number(lng)}]`Note: This mirrors the precision handling used for
aroundLatLngat line 35.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/instant-meilisearch/__tests__/assets/utils.ts(1 hunks)packages/instant-meilisearch/__tests__/geosearch.test.ts(2 hunks)packages/instant-meilisearch/src/adapter/search-request-adapter/__tests__/geo-rules.test.ts(1 hunks)packages/instant-meilisearch/src/adapter/search-request-adapter/geo-rules-adapter.ts(2 hunks)packages/instant-meilisearch/src/adapter/search-request-adapter/search-params-adapter.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/instant-meilisearch/__tests__/geosearch.test.ts (1)
packages/instant-meilisearch/__tests__/assets/utils.ts (1)
City(225-230)
packages/instant-meilisearch/src/adapter/search-request-adapter/__tests__/geo-rules.test.ts (1)
packages/instant-meilisearch/src/adapter/search-request-adapter/geo-rules-adapter.ts (1)
adaptGeoSearch(3-65)
packages/instant-meilisearch/src/adapter/search-request-adapter/search-params-adapter.ts (1)
packages/instant-meilisearch/src/adapter/search-request-adapter/geo-rules-adapter.ts (1)
adaptGeoSearch(3-65)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: autocomplete-client end-to-end-tests
- GitHub Check: instant-meilisearch end-to-end-tests
🔇 Additional comments (8)
packages/instant-meilisearch/__tests__/assets/utils.ts (1)
107-222: LGTM! GeoJSON support correctly added to test data.The implementation correctly adds
_geojsonfields alongside existing_geofields, properly following the GeoJSON Point format with coordinates in [longitude, latitude] order. The optional type annotation is appropriate since not all documents may have GeoJSON data.Also applies to: 229-229
packages/instant-meilisearch/src/adapter/search-request-adapter/geo-rules-adapter.ts (1)
14-30: Polygon precedence correctly implemented.The logic properly prioritizes
insidePolygonover other geo filters and includes robust validation:
- Filters invalid coordinate pairs
- Ensures at least 3 valid points for a valid polygon
- Short-circuits to avoid unnecessary processing
packages/instant-meilisearch/src/adapter/search-request-adapter/search-params-adapter.ts (1)
207-222: LGTM! Parameter correctly threaded through.The
insidePolygonparameter is properly destructured from the search context and passed toadaptGeoSearch, maintaining consistency with how other geo parameters are handled.packages/instant-meilisearch/src/adapter/search-request-adapter/__tests__/geo-rules.test.ts (1)
85-136: Excellent test coverage for polygon functionality.The test suite comprehensively validates:
- Polygon conversion for different shapes (triangle, quadrilateral)
- Precedence behavior ensuring
insidePolygontakes priority overinsideBoundingBoxand radius-based filters- Graceful handling of invalid input with fewer than 3 points
The expected filter format
_geoPolygon([lat, lng], ...)correctly matches Meilisearch's syntax.packages/instant-meilisearch/__tests__/geosearch.test.ts (4)
14-14: Filterable attributes correctly expanded for polygon support.Adding
'_geojson'to the filterable attributes alongside'_geo'enables polygon-based geo search. This aligns with Meilisearch v1.22.0's polygon support requirements.
112-133: LGTM! Polygon search test validates expected behavior.The test correctly verifies that documents within the defined polygon (Brussels) are included while documents outside (Paris) are excluded, demonstrating that the
insidePolygonparameter is properly translated into a working_geoPolygonfilter.
135-168: Good coverage of _geojson field requirement.This test correctly validates an important constraint: the
_geoPolygonfilter in Meilisearch only matches documents with the_geojsonfield. Documents with only_geoare properly excluded from polygon searches. The test includes appropriate cleanup of the test document.
170-202: Validates interoperability between _geo and _geojson fields.This test confirms that radius-based geo searches (using
_geoRadius) work with documents that have only_geojsonfields, demonstrating good backward compatibility in Meilisearch's geo search implementation. The test includes proper cleanup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/instant-meilisearch/src/adapter/search-request-adapter/geo-rules-adapter.ts (1)
14-30: Consider: Coordinate precision consistency.For consistency with
aroundLatLngprocessing (line 35, which uses.toFixed(5)), consider whether polygon coordinates should also be formatted to 5 decimal places. However, this may not be necessary if Meilisearch handles precision internally.If desired for consistency:
const formattedPoints = insidePolygon .map((pair) => { if (!Array.isArray(pair) || pair.length < 2) return null const lat = Number.parseFloat(String(pair[0])) const lng = Number.parseFloat(String(pair[1])) if (Number.isNaN(lat) || Number.isNaN(lng)) return null - return `[${lat}, ${lng}]` + return `[${lat.toFixed(5)}, ${lng.toFixed(5)}]` }) .filter((pt): pt is string => pt !== null)packages/instant-meilisearch/__tests__/geosearch.test.ts (1)
135-171: Document _geojson requirement for polygon geosearch
Polygon filters (insidePolygon/_geoPolygon) only work with GeoJSON shapes (_geojson). Update the geosearch docs (README or API reference) to note that documents must include a valid_geojsonfield to be returned in polygon searches (you can retain_geofor distance-based sorting).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/instant-meilisearch/__tests__/assets/utils.ts(1 hunks)packages/instant-meilisearch/__tests__/geosearch.test.ts(2 hunks)packages/instant-meilisearch/src/adapter/search-request-adapter/__tests__/geo-rules.test.ts(1 hunks)packages/instant-meilisearch/src/adapter/search-request-adapter/geo-rules-adapter.ts(2 hunks)packages/instant-meilisearch/src/adapter/search-request-adapter/search-params-adapter.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/instant-meilisearch/src/adapter/search-request-adapter/search-params-adapter.ts (1)
packages/instant-meilisearch/src/adapter/search-request-adapter/geo-rules-adapter.ts (1)
adaptGeoSearch(3-65)
packages/instant-meilisearch/__tests__/geosearch.test.ts (1)
packages/instant-meilisearch/__tests__/assets/utils.ts (1)
City(285-290)
packages/instant-meilisearch/src/adapter/search-request-adapter/__tests__/geo-rules.test.ts (1)
packages/instant-meilisearch/src/adapter/search-request-adapter/geo-rules-adapter.ts (1)
adaptGeoSearch(3-65)
🔇 Additional comments (8)
packages/instant-meilisearch/src/adapter/search-request-adapter/search-params-adapter.ts (1)
207-231: LGTM! Clean parameter threading.The addition of
insidePolygonto the destructuring and its pass-through toadaptGeoSearchcorrectly wires the new polygon geo-search capability. The change is minimal and aligns with the updated function signature.packages/instant-meilisearch/src/adapter/search-request-adapter/__tests__/geo-rules.test.ts (1)
85-136: LGTM! Comprehensive test coverage for polygon geo-search.The new test suite thoroughly validates
insidePolygonbehavior:
- Basic functionality with triangle and quadrilateral polygons
- Precedence over
insideBoundingBoxandaround*parameters- Graceful handling of invalid input (< 3 points)
The tests align well with the implementation and cover both happy paths and edge cases.
packages/instant-meilisearch/__tests__/assets/utils.ts (2)
285-290: LGTM! Correct GeoJSON Point structure.The addition of the optional
_geojsonfield correctly models a GeoJSON Point with coordinates in[lng, lat]order, which is the GeoJSON standard. The optional nature maintains backward compatibility with existing tests.
107-282: Coordinate consistency confirmed. All entries ingeoDatasethave matching_geoand_geojsoncoordinates.packages/instant-meilisearch/__tests__/geosearch.test.ts (3)
14-14: LGTM! Necessary filterable attribute addition.Adding
'_geojson'to the filterable attributes is required for polygon-based geo queries to function properly in Meilisearch.
112-133: LGTM! Well-structured polygon geo-search test.The test correctly validates
insidePolygonfunctionality by:
- Using a triangle polygon around the Brussels area
- Confirming Brussels is included in results
- Confirming Paris (far outside) is excluded
The polygon coordinates follow the
[lat, lng]convention used by InstantSearch.
173-205: LGTM! Good test coverage for _geojson compatibility.This test confirms that radius-based geo queries work with
_geojson-only documents, demonstrating interoperability between the two geo field formats. Proper cleanup is included.packages/instant-meilisearch/src/adapter/search-request-adapter/geo-rules-adapter.ts (1)
14-30: LGTM! Robust polygon validation and precedence logic.The implementation correctly:
- Validates polygon structure (array with ≥3 coordinate pairs)
- Parses and validates each coordinate pair, filtering invalid entries
- Ensures at least 3 valid points remain after validation
- Returns immediately to establish precedence over other geo filters
- Formats output consistent with existing
_geoRadiusand_geoBoundingBoxfiltersThe validation is thorough with appropriate NaN checks and defensive array length checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/instant-meilisearch/README.md (2)
794-794: Enhance documentation with reference link and clarify settings configuration.The
insidePolygondocumentation is clear, but could be improved:
- Add a link to Meilisearch's documentation on
_geojsonfor users who need to understand how to configure their index- Consider adding a brief note that the
filterableAttributessetting must be configured via the Meilisearch API or dashboard (before using this parameter)Apply this diff to add the documentation link:
-- `insidePolygon`: Filters search results to only include documents whose coordinates fall within a specified polygon. This parameter accepts an array of coordinate pairs `[[lat, lng], [lat, lng], ...]` that define the polygon vertices (minimum 3 points required). When `insidePolygon` is specified, it takes precedence over `insideBoundingBox` and `around*` parameters. **Note**: This feature requires `_geojson` to be added to your Meilisearch index's `filterableAttributes` setting. +- `insidePolygon`: Filters search results to only include documents whose coordinates fall within a specified polygon. This parameter accepts an array of coordinate pairs `[[lat, lng], [lat, lng], ...]` that define the polygon vertices (minimum 3 points required). When `insidePolygon` is specified, it takes precedence over `insideBoundingBox` and `around*` parameters. **Note**: This feature requires `_geojson` to be added to your Meilisearch index's [`filterableAttributes` setting](https://www.meilisearch.com/docs/reference/api/settings#filterable-attributes) (configured via the Meilisearch API or dashboard before performing searches).
821-839: Clear and well-structured example.The usage example effectively demonstrates the
insidePolygonfeature with:
- Clear comments about the prerequisite settings
- Proper coordinate array format
- Valid polygon (3 vertices forming a triangle)
One minor improvement: Consider clarifying that the
filterableAttributessetting must be configured using the Meilisearch settings API or dashboard before using this parameter, as it cannot be set directly in the InstantSearch configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/instant-meilisearch/README.md (1)
794-794: Improve readability by breaking up the long paragraph.The documentation for
insidePolygonis comprehensive but presented as a single 300+ character line, which impacts readability. Consider breaking it into a bulleted list.Apply this diff to improve readability:
-- `insidePolygon`: Filters search results to only include documents whose coordinates fall within a specified polygon. This parameter accepts an array of coordinate pairs `[[lat, lng], [lat, lng], ...]` that define the polygon vertices (minimum 3 points required). When `insidePolygon` is specified, it takes precedence over `insideBoundingBox` and `around*` parameters. Polygon filters require documents to contain a valid `_geojson` field with [GeoJSON format](https://geojson.org/). Documents without `_geojson` will not be returned in polygon searches, even if they have `_geo` coordinates. +- `insidePolygon`: Filters search results to only include documents whose coordinates fall within a specified polygon. + - Accepts an array of coordinate pairs: `[[lat, lng], [lat, lng], ...]` (minimum 3 points required) + - Takes precedence over `insideBoundingBox` and `around*` parameters when specified + - Requires documents to contain a valid `_geojson` field with [GeoJSON format](https://geojson.org/) + - Documents without `_geojson` will not be returned in polygon searches, even if they have `_geo` coordinates
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/instant-meilisearch/README.md(2 hunks)playgrounds/geo-javascript/index.html(1 hunks)playgrounds/geo-javascript/src/app.js(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- playgrounds/geo-javascript/index.html
🧰 Additional context used
🧬 Code graph analysis (1)
playgrounds/geo-javascript/src/app.js (4)
playgrounds/javascript/src/app.js (1)
search(4-13)packages/instant-meilisearch/src/client/instant-meilisearch-client.ts (1)
instantMeiliSearch(46-212)playgrounds/node-env/index.js (1)
instantMeiliSearch(4-8)playgrounds/react/src/App.jsx (1)
instantMeiliSearch(18-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: instant-meilisearch end-to-end-tests
🔇 Additional comments (4)
playgrounds/geo-javascript/src/app.js (3)
5-5: LGTM: Correct environment variable access for modern bundlers.The change from
process.env.GOOGLE_APItoimport.meta.env.GOOGLE_APIis appropriate for Vite or other modern bundlers that useimport.meta.envfor environment variables.
11-11: LGTM: Index name updated consistently.The index name change from
world_citiestoworld_cities_geojsonaligns with the PR's addition of polygon support and is consistently applied across all references.Also applies to: 23-31
13-13: Verify Meilisearch host and index accessibility. A direct check againsthttps://edge.meilisearch.com/indexes/world_cities_geojsonwith the existing API key returned 401. Ensure thatedge.meilisearch.comis the correct host, the API key has access, and theworld_cities_geojsonindex exists there.packages/instant-meilisearch/README.md (1)
820-834: LGTM: Clear and helpful polygon filtering example.The code example demonstrates
insidePolygonusage effectively, following the existing documentation patterns. The coordinates form a valid triangle around Brussels (matching the test data), and the reference to the geosearch documentation provides users with additional context.
|
@dureuill I know JS is not your main language, but it's more a review for the geosearch part, if relevant, to help Bruno who has less context on Geosearch. Tell me if not at all. |
|
The feature is ready but I need to do #1415 to update the geosearch playground |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (4)
playgrounds/geo-javascript/package.json (1)
7-9: vite is used in scripts but not declared.Add vite as a devDependency to avoid env‑specific failures.
Example:
"scripts": { "dev": "vite" }, + "devDependencies": { + "vite": "^5" + },Confirm the project’s Node/Vite constraints before pinning a specific minor.
playgrounds/geo-javascript/src/app.js (3)
3-13: Verify js-api-loader API usage; consider Loader pattern.Confirm that setOptions/importLibrary are valid named exports for @googlemaps/js-api-loader ^2.0.1 in your toolchain. If not, switch to the widely used Loader API.
Alternative:
import { Loader } from '@googlemaps/js-api-loader' const loader = new Loader({ apiKey: GOOGLE_MAP_API_KEY, version: 'weekly' }) loader.importLibrary('maps').then(() => { // init InstantSearch here })Optionally, I can check the package docs for the exact API for your version.
40-42: Showcase the new insidePolygon feature in the playground.Add insidePolygon to configure so users can see polygon filtering in action.
Apply this diff:
instantsearch.widgets.configure({ - hitsPerPage: 20, + hitsPerPage: 20, + // Demo: filter results within a polygon + insidePolygon: [ + [50.8466, 4.35], + [50.75, 4.1], + [50.65, 4.5], + ], }),(If this is deferred to #1415, feel free to skip here.)
55-71: Avoid highlighting numeric fields.helpers.highlight on population (numeric) is unnecessary; render raw value.
Apply this diff:
- <div class="hit-name"> - Population: {{#helpers.highlight}}{ "attribute": "population" }{{/helpers.highlight}} - </div> + <div class="hit-name"> + Population: {{population}} + </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
playgrounds/geo-javascript/.env(1 hunks)playgrounds/geo-javascript/index.html(2 hunks)playgrounds/geo-javascript/package.json(1 hunks)playgrounds/geo-javascript/src/app.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
playgrounds/geo-javascript/src/app.js (2)
playgrounds/javascript/src/app.js (1)
search(4-13)packages/instant-meilisearch/src/client/instant-meilisearch-client.ts (1)
instantMeiliSearch(46-212)
🪛 Gitleaks (8.28.0)
playgrounds/geo-javascript/.env
[high] 1-1: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
🔇 Additional comments (2)
playgrounds/geo-javascript/index.html (1)
35-35: Search box container class change — verify styling.Dropping the ais-SearchBox class from the container is usually fine (the widget injects its own classes). Just confirm no custom CSS relied on the container having that class.
playgrounds/geo-javascript/src/app.js (1)
14-20: Public demo key check for Meilisearch.Confirm the API key used here is a public demo key with strict origin/route limits. If not, move it to env and restrict it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a few comments on error handling an API keys. Otherwise I think we're good
| @@ -1 +0,0 @@ | |||
| GOOGLE_API=AIzaSyDOaUaar4GL0i99LpN2zQHzfWXL1wu_JQo | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did we revoke this api key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, but it's been public for years now.
And since this API key is intended for use in client-side JavaScript anyway, it's safe to share imo.
That said, @curquiza, if you want to revoke + recreate, you can.
| 'https://edge.meilisearch.com', | ||
| 'a63da4928426f12639e19d62886f621130f3fa9ff3c7534c5d179f0f51c4f303', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we have an URL + api key in the code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the current practice for most basic examples in our codebases
| if (Array.isArray(insidePolygon) && insidePolygon.length >= 3) { | ||
| const formattedPoints = insidePolygon | ||
| .map((pair) => { | ||
| if (!Array.isArray(pair) || pair.length < 2) return null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- what happens when we return
null? Is that ignoring the error silently? - are we silently ignoring extra coordinates rather than returning an error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, errors were silently ignored.
I updated the code to log warnings when coordinate pairs are malformed.
I chose this approach because this file already contains warnings for other ignored parameters:
console.warn(
'instant-meilisearch is not compatible with the `all` value on the aroundRadius parameter'
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/instant-meilisearch/src/adapter/search-request-adapter/geo-rules-adapter.ts(2 hunks)
| if (Array.isArray(insidePolygon) && insidePolygon.length >= 3) { | ||
| const invalidPairs: unknown[] = [] | ||
|
|
||
| const formattedPoints = insidePolygon | ||
| .map((pair) => { | ||
| if (!Array.isArray(pair) || pair.length < 2) { | ||
| invalidPairs.push(pair) | ||
| return null | ||
| } | ||
| const lat = Number.parseFloat(String(pair[0])) | ||
| const lng = Number.parseFloat(String(pair[1])) | ||
| if (Number.isNaN(lat) || Number.isNaN(lng)) { | ||
| invalidPairs.push(pair) | ||
| return null | ||
| } | ||
| return `[${lat}, ${lng}]` | ||
| }) | ||
| .filter((pt): pt is string => pt !== null) | ||
|
|
||
| if (invalidPairs.length > 0) { | ||
| console.warn( | ||
| 'instant-meilisearch: insidePolygon contains invalid coordinate pairs that were ignored:', | ||
| invalidPairs | ||
| ) | ||
| } | ||
|
|
||
| if (formattedPoints.length >= 3) { | ||
| filter = `_geoPolygon(${formattedPoints.join(', ')})` | ||
| return filter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
insidePolygon parsing drops required coordinates
InstantSearch sends insidePolygon as an array of polygons, and each polygon is a flattened list of lat/lng numbers (minimum six values). Here we iterate the top-level array as if each item were a single [lat, lng] pair, so we discard the remaining coordinates and never reach three points—formattedPoints stays < 3 and _geoPolygon(...) is never returned. In practice, any valid insidePolygon input from InstantSearch falls through to the legacy radius/bounding-box logic, so the new feature is effectively broken. Please chunk each polygon array in steps of two, parse every point, and handle the single-polygon shorthand (insidePolygon: number[]) before emitting _geoPolygon.(algolia.com)
- if (Array.isArray(insidePolygon) && insidePolygon.length >= 3) {
- const invalidPairs: unknown[] = []
-
- const formattedPoints = insidePolygon
- .map((pair) => {
- if (!Array.isArray(pair) || pair.length < 2) {
- invalidPairs.push(pair)
- return null
- }
- const lat = Number.parseFloat(String(pair[0]))
- const lng = Number.parseFloat(String(pair[1]))
- if (Number.isNaN(lat) || Number.isNaN(lng)) {
- invalidPairs.push(pair)
- return null
- }
- return `[${lat}, ${lng}]`
- })
- .filter((pt): pt is string => pt !== null)
-
- if (invalidPairs.length > 0) {
- console.warn(
- 'instant-meilisearch: insidePolygon contains invalid coordinate pairs that were ignored:',
- invalidPairs
- )
- }
-
- if (formattedPoints.length >= 3) {
- filter = `_geoPolygon(${formattedPoints.join(', ')})`
- return filter
- }
- }
+ if (Array.isArray(insidePolygon) && insidePolygon.length > 0) {
+ const polygons = (Array.isArray(insidePolygon[0]) && typeof insidePolygon[0] !== 'number'
+ ? insidePolygon
+ : [insidePolygon]) as unknown[]
+ const invalidPolygons: unknown[] = []
+
+ for (const polygon of polygons) {
+ if (!Array.isArray(polygon) || polygon.length < 6 || polygon.length % 2 !== 0) {
+ invalidPolygons.push(polygon)
+ continue
+ }
+
+ const points: string[] = []
+ for (let i = 0; i < polygon.length; i += 2) {
+ const lat = Number.parseFloat(String(polygon[i]))
+ const lng = Number.parseFloat(String(polygon[i + 1]))
+ if (Number.isNaN(lat) || Number.isNaN(lng)) {
+ invalidPolygons.push(polygon)
+ points.length = 0
+ break
+ }
+ points.push(`[${lat}, ${lng}]`)
+ }
+
+ if (points.length >= 3) {
+ filter = `_geoPolygon(${points.join(', ')})`
+ return filter
+ }
+ }
+
+ if (invalidPolygons.length > 0) {
+ console.warn(
+ 'instant-meilisearch: insidePolygon contains invalid coordinates that were ignored:',
+ invalidPolygons
+ )
+ }
+ }🤖 Prompt for AI Agents
packages/instant-meilisearch/src/adapter/search-request-adapter/geo-rules-adapter.ts
lines 15-43: the current code assumes each entry of insidePolygon is a [lat,lng]
pair, but InstantSearch may send insidePolygon as a single flattened number[]
(shorthand) or as an array of polygons where each polygon is a flattened list of
numbers; modify the parser to first detect if insidePolygon is an array of
numbers (single polygon shorthand) and wrap it as a single polygon array, then
for each polygon iterate in steps of two (chunk the flattened list into
[lat,lng] pairs), parse and validate each pair (collect invalid pairs), build
formattedPoints from all valid pairs across the polygon, and only emit
`_geoPolygon(...)` when you have at least three valid points; keep the
invalidPairs logging behavior but ensure you do not treat each top-level element
as a pair.
Pull Request
Related issue
Fixes #1412 and #1415
What does this PR do?
insidePolygonfrom instantSearch params_geoand_geojsoninteractPR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit
New Features
Bug Fixes / Stability
Tests
Documentation
Playgrounds