Skip to content
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

Consider dropping the default range #68

Closed
MoritzLost opened this issue Oct 24, 2022 · 5 comments
Closed

Consider dropping the default range #68

MoritzLost opened this issue Oct 24, 2022 · 5 comments
Labels
Feature Request New feature or request

Comments

@MoritzLost
Copy link

MoritzLost commented Oct 24, 2022

Right now, the only way to order an entry query by the distance to a custom location is to use a proximity search as mentioned in the docs. As far as I can tell, this always limits results to a specified range. There's no way to only order by distance without limiting the results to a specific radius around the target location.

I have a use-case where I want to order entries by their distance to a custom location, but don't want to exclude any entries from the query based on their distance. This looks like it should do the trick:

$results = Entry::find()
    ->myAddressField(['target' => ['lat' = 5, 'lng' => 5]]
    ->orderBy('distance')

But in reality, this will apply the default distance of 500 miles to the query and exclude any results that are farther away. This is unintuitive, and as far as I can tell the only way to prevent this is to set an arbitrarily large range like 9999 to prevent entries from being excluded. This is a bit ugly and might also result in worse performance (as opposed to not having a condition for the distance in the query at all).

I would argue in favour of dropping the unintuitive and arguably useless default of 500 miles. Instead, if the range is not specified, the query should only include the distance calculation, but not add any additional distance conditions.

@lindseydiloreto
Copy link
Collaborator

This is an interesting suggestion. The default range exists as a layer of protection, to ensure that people don't generate enormous queries. But perhaps it is overkill at this point.

We'll explore the idea of dropping the default range. It's probably pretty easy to do on a technical level.

However, we would probably hold off on releasing this until the next major version. It could be considered a breaking change, since some people may be relying on the default behavior.

Thanks for the recommendation. We'll do a little research, and probably make this change in v5.0.

@lindseydiloreto lindseydiloreto added the Feature Request New feature or request label Oct 28, 2022
@lindseydiloreto lindseydiloreto changed the title Allow for sorting by distance without performing a proximity search Consider dropping the default range Oct 28, 2022
@MoritzLost
Copy link
Author

MoritzLost commented Oct 31, 2022

Definitely a good candidate for a breaking change! I don't think too many people are relying on the implicit default – 500 miles is way too broad to meaningfully limit the results if you've got thousands of locations all over the world, and for stuff like local store locators it won't limit results at all. All of our sites using the plugin either limit the query by an explicit range or don't include a range at all as it's not relevant to the query.

We'll explore the idea of dropping the default range. It's probably pretty easy to do on a technical level.

I think it would just require dropping the default in ProximitySearchHelper::_applyProximitySearch and only adding the having or andWhere conditions if the range is explicitly passed.

@lindseydiloreto
Copy link
Collaborator

Great, thanks @MoritzLost. I believe you are correct about all of that. 🙂

@lindseydiloreto
Copy link
Collaborator

Good news, this is now the default behavior in v5.0.0. Thanks again for the suggestion!

@MoritzLost
Copy link
Author

@lindseydiloreto That's great, thanks Lindsey!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants