Skip to content

Fix _geo point not being correctly parsed #147

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

Merged
merged 8 commits into from
Feb 21, 2023
Merged

Fix _geo point not being correctly parsed #147

merged 8 commits into from
Feb 21, 2023

Conversation

bidoubiwa
Copy link
Contributor

@bidoubiwa bidoubiwa commented Dec 5, 2022

fixes: #143

If a user provides a field named _geo with a GeoPoint type, the type will be transformed into a Meilisearch geopoint compatible format.

In firestore:

{
    "_geo": { "latitude": x, "longitude": y }
}

In Meilisearch it becomes:

{
    "_geo": { "lng": x, "lat": y }
}

@oplik0 oplik0 mentioned this pull request Feb 18, 2023
3 tasks
@bidoubiwa bidoubiwa closed this Feb 20, 2023
@bidoubiwa bidoubiwa reopened this Feb 20, 2023
meili-bors bot added a commit that referenced this pull request Feb 20, 2023
154: Fix geo point with tests r=bidoubiwa a=oplik0

# Pull Request

## Related issue
Fixes #143 

## What does this PR do?
- this is a corrected version of #147 - the issue with tests was a Jest issue that appears to have been fixed in v28.
- While experimenting with this, I also updated the firebase packages - decided to leave them at newer versions instead of rolling these changes back
- Made some small changes to tests to align them with output format of `adaptFields`
- Re-added logging geopoint information to `adaptFields` to align with tests
- Added eslint-import-resolver-typescript, because without it the eslint import plugin doesn't work with `exports` in `package.json` which marked all `firebase-admin` imports as lint errors.

## PR checklist
Please check if your PR fulfills the following requirements:
- [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
- [x] Have you read the contributing guidelines?
- [x] Have you made sure that the title is accurate and descriptive of the changes?

Co-authored-by: oplik0 <[email protected]>
@bidoubiwa bidoubiwa marked this pull request as ready for review February 20, 2023 15:47
@bidoubiwa bidoubiwa added the skip-changelog The PR will not appear in the release changelogs label Feb 20, 2023
bidoubiwa and others added 6 commits February 20, 2023 16:52
@bidoubiwa bidoubiwa added breaking-change The related changes are breaking for the users skip-changelog The PR will not appear in the release changelogs bug Something isn't working and removed skip-changelog The PR will not appear in the release changelogs breaking-change The related changes are breaking for the users labels Feb 20, 2023
Copy link
Contributor

@alallema alallema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few questions, but Well Done!!!

@@ -0,0 +1,118 @@
'use strict'
/*
* Copyright 2022 Meilisearch
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to change this every year?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, I looked at other plugins and they don't have that disclaimer on top of the file. Im opining an issue to remove them

Copy link
Contributor

@alallema alallema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🎉

@bidoubiwa
Copy link
Contributor Author

bors merge

@meili-bors
Copy link
Contributor

meili-bors bot commented Feb 21, 2023

Build succeeded:

@meili-bors meili-bors bot merged commit d9dc3e6 into main Feb 21, 2023
@meili-bors meili-bors bot deleted the fix_geo_point branch February 21, 2023 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Meilisearch _geo consitency with with Firestore Geopoint
3 participants