-
Notifications
You must be signed in to change notification settings - Fork 31
fix: add Bolivian CI (Cédula de Identidad) validator #144
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
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @JulianoBazzi, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the library by adding support for the Bolivian Cédula de Identidad (CI). It provides a complete solution for validating, formatting, and compacting these identity numbers, ensuring data integrity and consistency for users dealing with Bolivian personal identification. The implementation includes detailed validation rules and is backed by a comprehensive suite of unit tests. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a validator for the Bolivian Cédula de Identidad (CI), complete with validation logic, formatting capabilities, and a comprehensive suite of unit tests. The new validator is correctly integrated into the project. My review identifies a minor issue with the regular expression in the format function and a discrepancy in one of the test cases regarding the expected error type. Apart from these points, the changes are solid.
| it('validate:1234567-1C', () => { | ||
| const result = validate('1234567-1C'); | ||
|
|
||
| expect(result.error).toBeInstanceOf(InvalidLength); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test case expects an InvalidLength error for the input 1234567-1C. However, the validation logic in ci.ts correctly identifies that '1C' is not a valid format for a department code (which must be two letters) and returns an InvalidFormat error. The implementation's behavior seems more accurate here. Please consider updating this test to expect InvalidFormat to align with the validation logic.
| const [value] = clean(input); | ||
|
|
||
| // Format: XXXXXXX-DD or XXXXXXX-DD-E | ||
| const match = value.match(/^(\d{7,8})([A-Z]{2})(\w{0,2})$/); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The \w character class in the regex is too permissive as it includes the underscore character (_), which is not a valid character for the extension part according to the validate function. To ensure consistency between format and validate, it's better to use a more specific character set.
| const match = value.match(/^(\d{7,8})([A-Z]{2})(\w{0,2})$/); | |
| const match = value.match(/^(\d{7,8})([A-Z]{2})([A-Z0-9]{0,2})$/); |
No description provided.