search

tastyigniter / TastyIgniter

:fire: Powerful, yet easy to use, open-source online ordering, table reservation and management system for restaurants
https://tastyigniter.com
MIT License
3.03k stars 988 forks source link

Move boundary precision to config #870

Closed ryanmitchell closed 3 years ago

ryanmitchell commented 3 years ago

Fixes #833

Currently setting('geocoder_boundary_precision') always returns NULL so precision is set to NULL which has unintended consequences flowing into the flame methods. This change sets the default of 8, while still allowing it to be configurable by a developer. IMO I don't think this needs to be a setting in the admin panel.

sampoyigi commented 3 years ago

IIRC, we have it there as a placeholder for when/if we decide to add it as an admin setting. Maybe set the default value to return when geocoder_boundary_precision is not available, instead of another config item when we already have one under the geocoder config.

ryanmitchell commented 3 years ago

Could do. Thought about that as the first step but got to thinking it's better as a config option rather than admin panel?

sampoyigi commented 3 years ago

Its already a config option under geocoder.precision and using the setting doesn't mean it has to be an admin setting option, all we are doing is giving it a default value which can still be changed from an extension using setting()->set('geocoder_boundary_precision', value).. There's just no point adding it under the system config when we already have it under geocoder...

ryanmitchell commented 3 years ago

Sure no problem - I just prefer env variables over admin panel settings, its personal preference. I’ll revert

On 13 Oct 2021, at 08:59, Sam Poyigi @.***> wrote:

Its already a config option under geocoder.precision and using the setting doesn't mean it has to be an admin setting option, all we are doing is giving it a default value which can still be changed from an extension using setting()->set('geocoder_boundary_precision', value).. There's just no point adding it under the system config when we already have it under geocoder...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tastyigniter/TastyIgniter/pull/870#issuecomment-942032297, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAMVOZLZRR774RN4MPMBZDUGU37HANCNFSM5F2FPKZA.