DivideBV / Postnl

Library to connect to PostNL's SOAP service called CIF
GNU General Public License v2.0
31 stars 40 forks source link

Enable Address::StreetHouseNrExt property #51

Closed annuh closed 7 years ago

annuh commented 7 years ago

The property StreetHouseNrExt is a combination of Street, HouseNr and HouseNrExt.

It isn't possible to use this property with this library, because there isn't a setter defined. There is a comment in this library about this, claiming this property isn't used: https://github.com/DivideBV/Postnl/blob/master/src/ComplexTypes/Address.php#L86-L91

But according to the official PostNL repo the field setStreetHouseNrExt CAN be used for NL, BE and DE addresses.: https://developer.postnl.nl/apis/labelling-webservice/documentation#toc-6.

I would like to use this property, because I think PostNL does a better job at extracting the required fields from an address correctly.

Is there a reason this property isn't used in this library?

ameenross commented 7 years ago

My reasoning for not using it is that it's unsafe to throw in arbitrary user input for an entire address to PostNL. A safer approach is used in almost all webshops or logistic systems. Either one of

annuh commented 7 years ago

Ok, I understand that it is safer to have separate inputs for all the address components. But some webshops (like ours) use a single input field to make it easier for the customers.

PostNL offers the option using the setStreetHouseNrExt. If it really didn't work they wouldn't support it. Besides, if we need to do the parsing manually I'm afraid it would result in more errors, because there are so many address formats (and it's not 100% clear what formats are accepted by PostNL).

Do you want to accept https://github.com/DivideBV/Postnl/pull/52? Otherwise I would need to use a fork to use the setStreetHouseNrExt property.

ameenross commented 7 years ago

Yes. Let's take the discussion there.