NOPE

[ThommyDude]

How does this look @PatricNox?
Also, next time we talk about a piece of code... Don't make me make a PR for it, you lazy bum 😂
The code can definitely be improved and/or cleaned a bit. But I'll leave that to you so it fits better with your standards.

[PatricNox]

Throw an error here instead. :)

throw new Error('Invalid Swedish Social Security Number');

[PatricNox]

Thanks for this PR!

However, some scenarios are having the unintended condition returned.

ssnIsValid('98-021-1-4976') // => true

thought should be false since both month and day parts are invalid.

however, ssnIsValid('1998-021-1-497') returns false like it should do. Ultimately meaning that the last-4 digit check works as intended. The same goes for ssnIsValid('98-02-1-1234') which would mark the day to be valided correctly.

So, therefore, it seems like the validation for month seems to not work.

These are the scenarios used for the check:

should eval true ✅ (4/4 tests passed)

ssnIsValid('199802111234')
ssnIsValid('9802111234')
ssnIsValid('1998-02-11-1234')
ssnIsValid('98-02-11-1234')

_should eval false ❌ (7/9 tests passed)

ssnIsValid('19980211123')
ssnIsValid('980211123')
ssnIsValid('98-021-1-1234') // => true
ssnIsValid('9802111234') // => true
ssnIsValid('98-02-1-1234')
ssnIsValid('98-02-1-1234')
ssnIsValid('98-2-11-1234')
ssnIsValid('1998-021-1-123')
ssnIsValid('1998-02-1-1234')

[ThommyDude]

First: The second question in your "Should eval true" and the fourth in the "Should eval false" are the same thing. So I'd like to confirm in which collumn that one is supposed to be, true or false? I'm assuming in true as there is nothing weird about that one.

Second: The 5th and 6th "Should eval false" are the exact same thing. This doesn't impact anything except that we're actually only running 8 tests that should be false, not 9.

Third: I'll take a look at if I can figure out an easy way to find out if the date is formatted correctly BEFORE we start changing the string.

[ThommyDude]

There, an update @PatricNox.
had to commit without the Lint verification because it didn't want to understand that I was commiting src/index.ts and instead kept giving me No files matching the pattern "src" were found. Which is cool, but that's not what I was committing 🤷🏻‍♂️

Also wanted to ask you something else. Shouldn't we move the first line of ssnMask "value = value.replace(/\W/g, '');" to below the call to ssnIsValid(value)? Because then you would still get the mistakes that my previous code gives because you'd send a already edited string to the function 🤔

[PatricNox]

First: The second question in your "Should eval true" and the fourth in the "Should eval false" are the same thing. So I'd like to confirm in which collumn that one is supposed to be, true or false? I'm assuming in true as there is nothing weird about that one.

ssnIsValid('9802111234') should be true, 98-02-11-1234, my mistake for putting it there. Sorry for that.

Second: The 5th and 6th "Should eval false" are the exact same thing

oops, again 😄

Shouldn't we move the first line of ssnMask "value = value.replace(/\W/g, '');" to below the call to ssnIsValid(value)?

the purpose of that regular expression was to clean up the string from any special characters. Though, now when I come to think about it, maybe that's a bad idea and it'll be better to use your condition work.

---

I've tested the new logic, ssnIsValid('98-021-1-4976') still evaluates to true but should be false.

I'd say we go for your approach to check if the string contains a dash, but then we should begin both methods to throw an exception incase there's any other character than a dash and alpha letters within the string.

value = value.replace(/\W/g, ''); can be deleted if we go towards your approach.

[ThommyDude]

Shouldn't we move the first line of ssnMask "value = value.replace(/\W/g, '');" to below the call to ssnIsValid(value)?

the purpose of that regular expression was to clean up the string from any special characters. Though, now when I come to think about it, maybe that's a bad idea and it'll be better to use your condition work.

Problem with that is that even in ssnMask the code only really works with the "cleaned" version of the ssn. All I'm saying is that we shouldn't clean it until we've verified that it's valid 😅

I've tested the new logic, ssnIsValid('98-021-1-4976') still evaluates to true but should be false.

Really? When I test locally it returns false, as expected 🤔

I'd say we go for your approach to check if the string contains a dash, but then we should begin both methods to throw an exception incase there's any other character than a dash and alpha letters within the string.

Instead of throwing an exception, we could use the regex that is currently used to "clean" the string to instead replace anything that isn't number into a -. (ex: value = value.replace(/\W/g, '-');) That way you can still have 1990-05-12:1234 but internally it would be seen as 1990-05-12-1234, which would work fine with the rest of the code.

value = value.replace(/\W/g, ''); can be deleted if we go towards your approach.

As stated above, I don't think we can delete it, we can only move it further down because ssnMask still requires the "cleaned" string to work correctly

EDIT/UPDATE:
Btw, should we be throwing errors or should we be returning false? Technically it's not an error when the ssn is invalid, because that's literally what we are testing for. So returning false in ssnIsValid when encountering an error is technically exactly what we want, isn't it? 🤔

[PatricNox]

@ThommyDude I agree, returning false makes more sense.

That way you can still have 1990-05-12:1234 but internally it would be seen as 1990-05-12-1234, which would work fine with the rest of the code.

Let's go with that!