Skip to content
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: IBAN validation #290

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ariskemper
Copy link
Contributor

No description provided.

Copy link

netlify bot commented Dec 9, 2023

‼️ Deploy request for valibot rejected.

Name Link
🔨 Latest commit cd196ad

@fabian-hiller
Copy link
Owner

Thanks for working on this! I will review it in the next few days. Another validator that's missing is card or creditCard. There is already an isLuhnAlgo utility implemented for this.

@fabian-hiller fabian-hiller self-assigned this Dec 9, 2023
@fabian-hiller fabian-hiller added the enhancement New feature or request label Dec 9, 2023
@ariskemper ariskemper marked this pull request as ready for review December 10, 2023 11:10
@ariskemper
Copy link
Contributor Author

@fabian-hiller will check isLuhnAlgo and add validator for credit cards.

@ariskemper
Copy link
Contributor Author

@fabian-hiller could you review it and merge this PR?

@fabian-hiller
Copy link
Owner

Next week is the last week of this semester and I have one exam left. That's why I'm not that fast with reviewing right now. I will try to do it over the weekend or next week.

@fabian-hiller fabian-hiller added the priority This has priority label Dec 15, 2023
@fabian-hiller
Copy link
Owner

fabian-hiller commented Dec 22, 2023

I am not an IBAN expert, but I suspect that your code does not detect a wrong IBAN in every case. For example, if you enter a letter instead of a number that happens to be converted to a valid number by (char.charCodeAt(0) - 55).toString(), isIBAN returns true.

I found validator.js on GitHub. Maybe we can get some inspiration from their isIBAN function.

@ariskemper ariskemper marked this pull request as draft December 23, 2023 11:41
@ariskemper
Copy link
Contributor Author

ariskemper commented Dec 23, 2023

@fabian-hiller could you give a case? This functon first check the length and then rearanges, so it moves first 4 chars at the end and after replaces chars from A-Z with a 2 digit number. If there would be 4 chars, it would replace those 4 with 2 digit number. On string of numbers then it perform mod97. I tested it against values from [validator.js] and i couldn't find any issue. (https://github.com/validatorjs/validator.js)
Validator.js has also whitelist and blacklist functionality.

@fabian-hiller
Copy link
Owner

Thank you for your feedback. I will check the code again.

@ariskemper ariskemper changed the title feat: add validation for IBAN feat: validation for IBAN Dec 26, 2023
@ariskemper ariskemper changed the title feat: validation for IBAN feat: IBAN validation Dec 26, 2023
@ariskemper ariskemper marked this pull request as ready for review December 26, 2023 21:56
@fabian-hiller fabian-hiller removed the priority This has priority label Jul 30, 2024
@fabian-hiller
Copy link
Owner

Unfortunately, due to our rewrite a few months ago, this PR is no longer up to date with our source code. I still plan to add this action, but other things have a higher priority for me at the moment. If you or anyone else in the community is interested in creating a new PR, that would be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants