Convert to ES6 JS with await; HTTPS; Deal with server down

[benbucksch]

Summary:

• Reformat as ES6 JS instead of Coffee script, removing further dependencies
• Support await
• Do simple syntax check first, before asking server
• If state server is down, return valid, if syntax check passes
• Use HTTPS

Reformat as ES6 JS instead of Coffee script, removing further dependencies

I don't really expect you to merge this. -- But you might, who knows. Coffee script annoyed me personally, because it was a dependency and a compilation.

await

As part of the ES6 rewrite, I also used a Promise, so the caller can simply await the call instead of using callbacks. Errors are thrown as normal exceptions.

Do simple syntax check first, before asking server

I've added a little regexp that checks the number a little further. There need to be between 2 and 13 numbers or letters in the VAT number.

If state server is down, return valid, if syntax check passes

Now, that one was annoying. Just when I worked on it, the German server was down. I verified with other VAT check pages, they also say "server down" for German VAT IDs. I'm actually glad I caught this during development rather than in production.

I don't want my corporate customers to be unable to buy my products just because the state can't keep its server up.

I worked around it by returning valid in such cases, and added another return value, in case any caller wants to only accept really server-validated numbers or add special handling.

The syntax validation helps a little bit in this case.

HTTPS

The old code (probably survived from 2013) used a non-SSL HTTP URL. That's no longer appropriate in 2019. Luckily, the server accepts HTTPS, so I changed the URL to that.

API changes

• Pass in full VAT ID as single string, e.g. 'FR60421938861', instead of separate country and number.
• Trim spaces in name and address fields. I got such garbage e.g. for FR60421938861.
• Do not return name and address as '---', but empty string.

Obviously, this is a breaking API change, due to promises.

[benbucksch]

fixes #16 #26

[thebillg]

thanks for the PR, will review soon

[koesper]

I've done a new implementation based on @benbucksch's version. Seems to work fine. Serverdown works as expected, and i've seen correct answers to both valid and invalid VAT numbers.

Only when i enter a number that cant be parsed, I get an exception, instead of a regular response. Try it with one of these in test.js

    var vatID = "DE000000000";     // invalid but correct result
    var vatID = "DE000000000/B01"; // invalid but exception Error: The VAT number part is empty or invalid
    var vatID = "XX183362587";     // invalid but exception Error: The country code in the VAT ID is invalid

Not sure if that is the expected behaviour or not. It is fine for me, but i personally expected a json with something like this:

{
  countryCode: ''l, // unknown country code 
  vatNumber: '', // could not parse vat number
  valid: false,
  serverValidated: false,
  name: '',
  address: ''
}

[benbucksch]

Yes, instead of throwing an exception, we could return an object with valid: false and an error message. That should be simple to fix.

FWIW, the old code returned an Error object to the callback in place of the normal result object: return callback(new Error(ERROR_MSG['INVALID_INPUT']));. I prefer to not return 2 different types in the same parameter. There are many possible solutions, but I prefer 2 options:

• Throw an exception immediately to the caller, not in the callback. (This is what my patch does, because the caller has to handle exceptions anyway, no matter what.)
• Expand the result object with an error message. It would return valid: false, errorMessage: 'The country code does not exist'. (This is your suggestion.)

I'll be waiting for the general feedback from the maintainer before touching this patch again.

[benbucksch]

(Last comment updated)

[benbucksch]

Bill G, are you otherwise OK with these changes? Including the change from CoffeeScript to ES6 and the breaking API changes that require to adapt the caller?

[koesper]

@benbucksch Any idea when you will be able to work on this?

[koesper]

We were so close to greatness with this pullrequest! Can you finalize it @benbucksch ?

[benbucksch]

yes, if be happy to. but I'm only vacation and will have plenty of work when i come back. please ping me again in 3 weeks.

Am 5. Februar 2020 11:48:14 MEZ schrieb Casper de Groot [email protected]:

We were so close to greatness with this pullrequest! Can you finalize it @benbucksch ?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/viruschidai/validate-vat/pull/28#issuecomment-582349410

[viruschidai]

Rewrite it in TypeScript https://github.com/viruschidai/validate-vat-ts

[zedrdave]

Any update? This lib looks nice, but not having Promise support is a bit of a showstopper… :-/

[koesper]

For me, the rewritten version @viruschidai mentions https://github.com/viruschidai/validate-vat/pull/28#issuecomment-582678144 is working perfectly, as a drop-in-replacement. Give it a try!

[benbucksch]

@zedrdave : Can you see whether the version here in the pull request works for you? This PR does implement a Promise API.

[zedrdave]

@koesper ah ok. I didn't quite understand that this was an announcement (sounded like a request to rewrite it). Thanks, I'll try!

@viruschidai maybe would be a good idea to post a notice at the top of the README…

[zedrdave]

@benbucksch Sorry, I just realised that apparently this package does not work in the browser… Which is where I need it :-/

[benbucksch]

@viruschidai : I've addressed most of your review requests and those of @thebillg

[red15]

I believe the scope of this PR has gone way beyond a small change dealing about state servers not being available. I think you @benbucksch can agree with that since you've already renamed and version bumped your branch several times. I would say you have successfully created a fully fledged fork and should just rename your repo and have the credits mention this as the original project.

Best of luck with maintaining the newly forked library 😄 May your bugs be few and well documented/reproduce-able and your received PR's small and clean (unlike this one 😛 ) It would be courteous to close this PR yourself to avoid forcing @viruschidai to have to close it, making it look like he closed it out of spite.