-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add contains tag #14
Add contains tag #14
Conversation
WalkthroughThe changes introduce a new validation rule called "contains" for checking if a value includes a specified substring. This is supported by updates to the documentation, error handling, and localization files. Additionally, new test cases have been added to ensure the functionality works correctly, and a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CSVDecoder
participant Validator
participant ErrorHandler
User->>CSVDecoder: Submit CSV with "contains" validation
CSVDecoder->>Validator: Validate "contains" rule
Validator->>Validator: Check substring presence
alt Substring found
Validator-->>CSVDecoder: Validation success
else Substring not found
Validator-->>ErrorHandler: Return ErrContainsID
ErrorHandler-->>CSVDecoder: Show error message
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Code Metrics Report
Details | | main (9133095) | #14 (212993e) | +/- |
|---------------------|----------------|---------------|-------|
+ | Coverage | 81.9% | 83.1% | +1.2% |
| Files | 6 | 6 | 0 |
| Lines | 344 | 357 | +13 |
+ | Covered | 282 | 297 | +15 |
+ | Test Execution Time | 8s | 1s | -7s | Code coverage of files in pull request scope (83.6% → 84.4%)
Reported by octocov |
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.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
parser.go (1)
138-146
: Approved: Addition ofcontains
tag handling inparseValidateTag
.The implementation for handling the
contains
tag inparseValidateTag
is correctly integrated with the existing validation logic. The use ofparseOneOf
and the subsequent checks ensure that the tag is correctly parsed and validated.Consider adding a comment before the
contains
case to explain that it expects exactly one value, similar to the comments in other cases. This will improve code readability and maintainability.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- README.md (1 hunks)
- csv_test.go (1 hunks)
- errors.go (1 hunks)
- i18n/en.yaml (1 hunks)
- i18n/ja.yaml (1 hunks)
- i18n/ru.yaml (1 hunks)
- parser.go (1 hunks)
- tag.go (1 hunks)
- validation.go (1 hunks)
Additional context used
golangci-lint
csv_test.go
589-589: singleCaseSwitch: should rewrite switch statement to if statement
(gocritic)
618-618: singleCaseSwitch: should rewrite switch statement to if statement
(gocritic)
Additional comments not posted (11)
i18n/ja.yaml (2)
79-80
: Translation forErrContains
is correct.The translation "指定された値を含んでいません" accurately conveys the intended message in Japanese.
82-83
: Translation forErrInvalidContainsFormat
is correct.The translation "'contains'タグの形式が無効です" accurately conveys the intended message in Japanese.
i18n/en.yaml (2)
73-74
: Translation forErrContains
is correct.The translation "target does not contain the specified value" accurately conveys the intended message in English.
76-77
: Translation forErrInvalidContainsFormat
is correct.The translation "'contains' tag format is invalid" accurately conveys the intended message in English.
i18n/ru.yaml (2)
73-74
: Translation forErrContains
is correct.The translation "целевое значение не содержит подстроку" accurately conveys the intended message in Russian.
76-77
: Translation forErrInvalidContains
is correct.The translation "Формат тега 'contains' недопустим" accurately conveys the intended message in Russian.
tag.go (1)
53-54
: Approved: Addition ofcontainsTagValue
constant.The addition of the
containsTagValue
constant is consistent with the existing structure and naming conventions used in the file.errors.go (1)
101-104
: Approved: Addition of error identifiersErrContainsID
andErrInvalidContainsFormatID
.The addition of the error identifiers
ErrContainsID
andErrInvalidContainsFormatID
is consistent with the existing structure and naming conventions used in the file.README.md (1)
132-132
: Documentation for "contains" validation rule is clear and informative.The addition of the "contains" validation rule in the documentation is well-explained and includes an appropriate example. This should help users understand how to use this new validation feature effectively.
csv_test.go (2)
569-596
: Test case for "validate contains" is well-implemented.The test case effectively checks the functionality of the "contains" validation rule, ensuring that it correctly identifies when the specified substring is not present. The error message is clear and provides useful information for debugging.
Tools
golangci-lint
589-589: singleCaseSwitch: should rewrite switch statement to if statement
(gocritic)
598-625
: Test case for "invalid contains tag format" is effective.This test case successfully verifies the system's ability to handle malformed validation tags. The error message clearly specifies the issue, aiding in quick identification and resolution of format errors in validation tags.
Tools
golangci-lint
618-618: singleCaseSwitch: should rewrite switch statement to if statement
(gocritic)
// containsValidator is a struct that contains the validation rules for a contains column. | ||
type containsValidator struct { | ||
contains string | ||
} | ||
|
||
// newContainsValidator returns a new containsValidator. | ||
func newContainsValidator(contains string) *containsValidator { | ||
return &containsValidator{contains: contains} | ||
} | ||
|
||
// Do validates the target contains the contains value. | ||
func (c *containsValidator) Do(localizer *i18n.Localizer, target any) error { | ||
v, ok := target.(string) | ||
if !ok { | ||
return NewError(localizer, ErrContainsID, fmt.Sprintf("value=%v", target)) | ||
} | ||
|
||
if !strings.Contains(v, c.contains) { | ||
return NewError(localizer, ErrContainsID, fmt.Sprintf("contains=%s, value=%v", c.contains, target)) | ||
} | ||
return nil | ||
} |
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.
Implementation of containsValidator
is robust and well-structured.
The containsValidator
struct and its associated methods are correctly implemented to support the new "contains" validation rule. The Do
method effectively checks for the presence of the specified substring and returns informative error messages when the validation fails.
Suggestion for Improvement:
Consider enhancing the error message in the Do
method to include more context about the expected and actual values, which could aid in debugging complex issues.
Summary by CodeRabbit
New Features
Bug Fixes
Localization