This module is great and I have achieved the necessary level of filtering by blacklisting all none allowed post codes from my city.

However when in whitelist mode it allows all of the city postcodes and doesn't check the numeric part.

Comments

andymantell’s picture

andymantell commented

Hi Joe,

Could you post a screenshot of the admin panel config screen, and also upload a text file with the postcodes you are using so I can test with the same data as you? Or if you could dump the entire contents of the commerce_postal_code_filter_config variable that would be fine too.

Thanks,

Andy

joe huggans’s picture

joe huggans
Primary language English
Location Kiel, Germany
commented

Hi Andy,

My settings were as follows, whitelist with these postcodes: LS1, LS2, LS3, LS4, LS5, LS6, LS7, LS8, LS9, LS12, LS13, LS16, LS17, with only shipping enabled in the 'Addressees to filter', billing is left unchecked.

This simply allows all LS postcodes rather than just the numeric versions of them. My method to get it working was to blacklist all other LS postcodes. And a shame because I can not use the check postcode block on my home page and it is a really nice feature.

You should also be able to recreate an error if you use this setup but then checkout with a blacklisted postcode such as LS11, but only using the just the billing as a shipping profile requirement to be filled out.

joe huggans’s picture

joe huggans
Primary language English
Location Kiel, Germany
commented

If you need any more help andy I would be willing to help out, maybe I can take a look at this bug at some point in the next week.

andymantell’s picture

andymantell commented

Hi Joe,

If you have any thoughts as to what it might be then that would be great. Unfortunately I just haven't the time at the moment to take much of a look. I assume that there's something not quite right in _commerce_postal_code_filter_postal_code_is_valid() - What I really want to do is to write failing tests every time someone raises a ticket, and then fix the test, but, time!

Andy

andymantell’s picture

andymantell commented
Category: Feature request » Bug report
Priority: Normal » Critical

Hi Joe,

I see what's happening here. It basically amounts to a rather naive check in _commerce_postal_code_filter_postal_code_is_valid whereby it uses strpos to detect whether the submitted postcode *starts with* one of the items in the whitelist. However, that doesn't actually make sense because LS11 isn't merely a more specific postcode under the LS1 area, it is an area in it's own right.

I will have a think about how to fix this as it's obviously fairly fundamental!

joe huggans’s picture

joe huggans
Primary language English
Location Kiel, Germany
commented

ahh well spotted, I will try take a look at this later when I also have some time! Thanks Andy!

andymantell’s picture

andymantell commented

I think the solution will be to kill off the use of strpos entirely, certainly for UK postcodes. And then use something like this to validate and split the postcode entered by the user into it's two component parts, and perform some comparisons on the parts. The current use of strpos is just plain wrong at the end of the day. It sorta approximates the solution and gets it right some of the time, but it also gets it wrong a lot too.

joe huggans’s picture

joe huggans
Primary language English
Location Kiel, Germany
commented

I agree, this solution looks good! And yes the strpos is not ideal for this circumstance because it can not distinguish between ls11 or ls1 for example.

joe huggans’s picture

joe huggans
Primary language English
Location Kiel, Germany
commented

Hi Andy,

Had a quick look.

Can we not use preg_match rather than strpos ? This should search for the entire string rather than just whether it exists in part within another string.

Joe

andymantell’s picture

andymantell commented

I think it's more complex than that isn't it? What would the regular expression be? Whether by regular expression or strpos, If I search for ls1 in a postcode beginning with ls11 you would still get an erroneous match.

I think the parts of the postcode need to be split up and treated differently probably. Once you've split the postcode up (assuming we can do so reliably) you can just say, is the first part *equal to* the passed in value. No need to search the string, the problem becomes simple once you've done that split. Not sure about the second half of the postcode for the moment, I'll have to think about how they behave. Maybe we just say that we don't support matching based on the 2nd half given that they represent such small geographic areas.

joe huggans’s picture

joe huggans
Primary language English
Location Kiel, Germany
commented

I would probably say in the documentation that the module only supports postcodes based on the first half as anything more than this would probably not be necessary anyway.

If it were possible to limit the postcode to 3-4 characters would it then be as simple as looping through them all and seeing if any of them actually equal the postcode in question rather than having to use a function like stringpos or pregmatch?

andymantell’s picture

andymantell commented

yeah I think this is going along the right lines. Restricting to the first half of the postcode. We'd need to wrao this all in a special case for UK. Although really probably need to investigate the mechanics of US zip codes etc and see how they work too.

Once you've got the first half of the postcode isolated, it is as you say, a simple check for equality...

joe huggans’s picture

joe huggans
Primary language English
Location Kiel, Germany
commented

To split the post code, I do believe that all postcodes end with 3 characters.

andymantell’s picture

andymantell commented

Agreed. Wikipedia says:

The inward part is the part of the postcode after the single space in the middle. It is three characters long. The inward code assists in the delivery of post within a postal district. Examples of inward codes include "0NY", "7GZ", "7HF", or "8JQ".[24][24]
from: http://en.wikipedia.org/wiki/Postcodes_in_the_United_Kingdom#Inward_code

andymantell’s picture

andymantell commented

Still a little tricky I suppose as you don't know that the person has entered a full postcode... I still think this is probably the best bet:

https://www.craigfrancis.co.uk/features/code/phpPostCode/

Whatever ends up being done, I think we need to get a test suite together. Unfortunately I'm flat out at the moment so I don't think I'm going to be able to address this issue any time soon.

joe huggans’s picture

joe huggans
Primary language English
Location Kiel, Germany
commented

This seems to work.. I am now able to use the whitelist mode rather than the blacklist mode. 

// This removes the second part of the post code
$postal_code = substr($postal_code, 0, strlen($postal_code)-3);
  
  foreach ($postal_codes as $filtered_item) {
   // Now able to check directly rather than using strpos
    if ($postal_code == $filtered_item) {

      return ($mode == 'blacklist') ? FALSE : TRUE;
    }
  }
joe huggans’s picture

joe huggans
Primary language English
Location Kiel, Germany
commented

Yea it's cool, I have it working how I need, I will try test things out and hopefully help out the best I can, might have some spare time soon anyway.

There needs to be a check to see whether postcode is a full postcode I guess also. Maybe this can be done at the beginning of the _commerce_postal_code_filter_postal_code_is_valid function

andymantell’s picture

andymantell commented

Glad that is working for you. If someone enters a non-full postcode though that will still fail I think as your strlen-3 bit assumes the full postcode

andymantell’s picture

andymantell commented

I guess if you have any time to work on this, can you fork this github repo, and submit PRs instead of patch files? The Drupal.org Git workflow is dire.

joe huggans’s picture

joe huggans
Primary language English
Location Kiel, Germany
commented

Thanks Andy, yes I'm just getting into this side of Drupal so happy to learn and help in any way that I can right now. Thanks for the link.

deepak_mishra’s picture

deepak_mishra commented

Hi,

I need Commerce Postal Code Filter module in Drupal 8 version.
my issue when user fill specific postal code then only shipping pf product are available.
example: country: India, state: Maharashtra, city: Mumbai, postal code: 400060 then if user type postal code: 400069 then shipping are not available.

andymantell’s picture

andymantell commented

@deepak_mishra This module is currently unsupported, not in active development. No Drupal 8 release is therefore planned, especially since I believe the fundamental logic around postcode filtering is just _wrong_. I would suggest you fork the code and modify it to suit your needs.