Skip to content

Add GeoBoundingBox filter#84

Merged
Toflar merged 7 commits intoloupe-php:mainfrom
alexander-schranz:feature/geo-bounding-box-filter
Sep 18, 2024
Merged

Add GeoBoundingBox filter#84
Toflar merged 7 commits intoloupe-php:mainfrom
alexander-schranz:feature/geo-bounding-box-filter

Conversation

@alexander-schranz
Copy link
Copy Markdown
Contributor

@alexander-schranz alexander-schranz commented Sep 17, 2024

Why Loupe already supports filtering by a GeoDistance. Which support we already merged also int SEAL. It would also be nice to filter by a bounding box (rectangle). That is mostly used for things when a user navigates around a map and have a specific area shown.

See fore more information: #83

fixes #83

Implemented as far as I could get with my knowledge.

Comment thread tests/Functional/SearchTest.php Outdated
Comment thread src/Internal/Filter/Ast/GeoBoundingBox.php Outdated
Copy link
Copy Markdown
Contributor

@Toflar Toflar left a comment

Choose a reason for hiding this comment

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

Implemented as far as I could get with my knowledge.

Do you want me to take over from here or do you want to give it a shot with my feedback? You are almost there!

Comment thread src/Internal/Filter/Ast/GeoBoundingBox.php Outdated
Comment thread tests/Functional/SearchTest.php Outdated
Comment thread src/Internal/Filter/Parser.php Outdated
@alexander-schranz alexander-schranz force-pushed the feature/geo-bounding-box-filter branch from 5f10424 to dc88b89 Compare September 18, 2024 07:59
@alexander-schranz alexander-schranz marked this pull request as ready for review September 18, 2024 08:00
@alexander-schranz alexander-schranz force-pushed the feature/geo-bounding-box-filter branch from fc86f25 to bcf1922 Compare September 18, 2024 08:16
@alexander-schranz
Copy link
Copy Markdown
Contributor Author

@Toflar ready for review

Copy link
Copy Markdown
Contributor

@Toflar Toflar left a comment

Choose a reason for hiding this comment

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

Awesome! You have implemented almost all that's needed, great!!

Comment thread src/Internal/Search/Searcher.php
Comment thread tests/Functional/SearchTest.php Outdated
Comment thread tests/Unit/Internal/Filter/ParserTest.php
Comment thread src/Internal/Filter/Ast/GeoBoundingBox.php Outdated
Copy link
Copy Markdown
Contributor

@Toflar Toflar left a comment

Choose a reason for hiding this comment

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

Just one more test I think would be very helpful.
This looks very cool - how do you like it? :)

'_geoBoundingBox(location, 1.00 2.00, 2.00, 3.00)',
"Col 31: Error: Expected ',', got '2.00'",
];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just one more test for invalid coordinates would be great :)

@Toflar
Copy link
Copy Markdown
Contributor

Toflar commented Sep 18, 2024

Oh and some docs on how to use the filter https://github.com/loupe-php/loupe/blob/main/docs/searching.md#filter

@alexander-schranz alexander-schranz force-pushed the feature/geo-bounding-box-filter branch from 9150db5 to df140d9 Compare September 18, 2024 11:54
Copy link
Copy Markdown
Contributor

@Toflar Toflar left a comment

Choose a reason for hiding this comment

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

Some docs improvements and then I think this is ready to be merged. Awesome work, Alex!

Comment thread docs/searching.md Outdated
Comment thread docs/searching.md Outdated
Comment thread docs/searching.md Outdated
Comment thread docs/searching.md Outdated
Co-authored-by: Yanick Witschi <yanick.witschi@terminal42.ch>
@Toflar Toflar merged commit 54c0548 into loupe-php:main Sep 18, 2024
@Toflar
Copy link
Copy Markdown
Contributor

Toflar commented Sep 18, 2024

Thanks @alexander-schranz! This is very cool! I'm going to clean up some stuff and then release it as v0.7 :)

@alexander-schranz alexander-schranz deleted the feature/geo-bounding-box-filter branch September 18, 2024 12:18
@alexander-schranz
Copy link
Copy Markdown
Contributor Author

@Toflar Awesome. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for filtering by a GeoBoundingBox

2 participants