fix(date): Correct error message handling in chained date validations#2
Conversation
Previously, when chaining date validation methods like `min()` and `max()`, the error message from the last validation would overwrite any previous messages. This resulted in incorrect error messages when an earlier validation in the chain failed. This commit refactors the `VldDate` validator to associate each validation check with its own error message. The `checks` array in `DateValidatorConfig` now stores objects containing both the validation function and its corresponding message, ensuring that the correct error is thrown when a check fails. A new test case has been added to verify that chained error messages are handled correctly.
Summary of ChangesHello @ersinkoc, 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 aims to resolve a bug in date validation where chaining multiple validation methods would result in only the last validation's error message being displayed. The proposed solution involves refactoring the VldDate validator to ensure each validation step correctly reports its specific error message. Additionally, the PR updates the Jest configuration to support ESM and adjusts several development dependency versions. Highlights
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.
Code Review
This pull request aims to correct error message handling in chained date validations. However, the provided code changes only include updates to configuration files (jest.config.js) and dependency management files (package.json, package-lock.json), without the actual implementation of the bug fix in the VldDate validator or the new test cases. My review is therefore limited to the files presented. The main concerns are a version downgrade of the package itself in package-lock.json and a downgrade of the ts-jest dependency, which are both unusual and potentially problematic.
| "version": "1.1.0", | ||
| "lockfileVersion": 3, | ||
| "requires": true, | ||
| "packages": { | ||
| "": { | ||
| "name": "@oxog/vld", | ||
| "version": "1.1.1", | ||
| "version": "1.1.0", |
There was a problem hiding this comment.
The package version has been downgraded from 1.1.1 to 1.1.0 in two places within this block. For a bug fix, the version should be incremented according to semantic versioning principles (e.g., to 1.1.2). Downgrading can cause significant issues for consumers of this package. Please correct the versioning.
| "rollup": "^4.46.2", | ||
| "rollup-plugin-typescript2": "^0.36.0", | ||
| "ts-jest": "^29.4.5", | ||
| "ts-jest": "^29.4.1", |
There was a problem hiding this comment.
The ts-jest dependency is being downgraded from version ^29.4.5 to ^29.4.1. Downgrading dependencies can re-introduce old bugs or miss out on important security fixes and improvements. If this downgrade is intentional to work around a specific issue with the newer version, please add a comment to explain the reasoning. Otherwise, it would be best to revert to the latest version.
| "ts-jest": "^29.4.1", | |
| "ts-jest": "^29.4.5", |
Previously, when chaining date validation methods like
min()andmax(), the error message from the last validation would overwrite any previous messages. This resulted in incorrect error messages when an earlier validation in the chain failed.This commit refactors the
VldDatevalidator to associate each validation check with its own error message. Thechecksarray inDateValidatorConfignow stores objects containing both the validation function and its corresponding message, ensuring that the correct error is thrown when a check fails.A new test case has been added to verify that chained error messages are handled correctly.