-
Notifications
You must be signed in to change notification settings - Fork 358
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
Feat: configurable postcode resolver #2021
base: 1.x
Are you sure you want to change the base?
Feat: configurable postcode resolver #2021
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Sounds like a good idea. I'll review with @alecritson when he's back from annual leave. |
I had a little look at this, my 15 minutes of thought is, instead of making this config specific, why not make it country specific? This way we just need to register new postcode resolvers for countries and let the core handle it... We already have a So instead of this: $postcodeParts = (new PostcodeResolver)->getParts(
$this->postcodeLookup->postcode
);
$query->whereIn('postcode', $postcodeParts); We have this: $query->whereIn('postcode', $this->postcodeLookup->getParts()); The public function getParts(): array
{
// New facade? Could resolve to a postcode manager class
return Postcode::country($this->country)->getParts($this->postcode);
} On this new manager class we could have class PostcodeManager
{
protected array $resolvers = [];
public function addResolver(string $class)
{
$this->resolvers[] = $class;
}
public function country(Country $country)
{
foreach ($this->resolvers as $resolver) {
// Check the resolver works for this country and return it...
}
}
} Need to think about if multiple resolvers are registered, however we could have some sort of key signature on the array or make it so the last in the array is the "first" so the default will always come last in the matching 🤷 A postcode resolver could look like: class PostcodeResolver implements PostcodeResolveInterface
{
protected array $countries = ['GB'];
public function supportsCountry(Country $country): bool
{
// The default one will just return true
return in_array($country->iso2, $this->countries);
}
// ...
} Then maybe to register a new resolve you just do: // Again, new facade?
Postcode::addResolver(MyCustomResolver::class); Idk this was my "thinking out loud" 🤔 |
I like the idea postcode resolvers. The only thing I would add on is to allow adding the supported countries to a resolver |
postcode format varies for each countries. it will be hard to only have 1 lunar resolver to cater for all
this PR enables dev to replace with own resolver that suit the countries the store is running