Conversation
…length to handle arrays and slices
…length to handle arrays and slices
|
Caution Review failedThe pull request is closed. WalkthroughThe changes update the validator functions to support strings, slices, and arrays by using reflection to determine the data type. In Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant MinLengthFunc
participant Reflect
Caller->>MinLengthFunc: Call MinLength(value, minLength)
Note over MinLengthFunc: Check if value is nil
MinLengthFunc->>Reflect: Determine type (string, slice, array)
Reflect-->>MinLengthFunc: Return kind and length
alt Valid type (string/slice/array)
MinLengthFunc->>Caller: Validate length meets minLength
else Invalid type
MinLengthFunc->>Caller: Return error (unexpected type)
end
sequenceDiagram
participant Caller
participant ValidateFunc
participant ValidatorFn
Caller->>ValidateFunc: Validate(data, options)
ValidateFunc->>ValidatorFn: Execute validator
ValidatorFn-->>ValidateFunc: Return error result (with or without Message)
alt Custom message exists
ValidateFunc->>Caller: Return formatted error using Message
else No custom message
ValidateFunc->>Caller: Return original error
end
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 golangci-lint (1.64.8)level=warning msg="[runner] Can't run linter goanalysis_metalinter: buildir: failed to load package : could not load export data: no export data for "github.com/gin-gonic/gin"" 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
validator/range_test.go (1)
1-474: 💡 Verification agent🧩 Analysis chain
Fix pipeline test failures related to error messages.
The pipeline is failing because the test expects error messages to include the element index, but the implementation of
EachWithOptionsin range.go was changed to return errors directly without the index.You need to either:
- Update the expected error messages in the tests to match the new error format
- Or revert the change in range.go line 157 to maintain the index in error messages
🏁 Script executed:
#!/bin/bash # Check the failing tests to confirm the error go test ./validator -v -run TestEachWithOptions/invalid_array_-_missing_required_fieldLength of output: 707
Fix dependency configuration and error message mismatches in tests
- The pipeline is failing due to missing go.sum entries for packages (e.g. “golang.org/x/text/cases” and “golang.org/x/text/language”). Please update the dependency configuration (e.g. via
go getorgo mod tidy) so that the build setup passes.- Additionally, the tests for
EachWithOptionsnow expect error messages with an element index, but the current implementation inrange.go(line 157) returns error messages without it. Decide whether to update the expected error messages in the tests or revert the change inrange.goto include the index.🧰 Tools
🪛 GitHub Actions: Go Tests
[error] 253-253: TestEachWithOptions/invalid_array_-_missing_required_field: Error message mismatch. Expected: 'element at index 1: name is required', Actual: 'name is required'.
[error] 253-253: TestEachWithOptions/invalid_array_-_wrong_type: Error message mismatch. Expected: 'element at index 1: value must be a string', Actual: 'value must be a string'.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
validator/range.go(2 hunks)validator/range_test.go(1 hunks)validator/validator.go(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
validator/range.go (1)
validator/validator.go (1) (1)
ValidatorFunc(10-10)
🪛 GitHub Actions: Go Tests
validator/range_test.go
[error] 253-253: TestEachWithOptions/invalid_array_-_missing_required_field: Error message mismatch. Expected: 'element at index 1: name is required', Actual: 'name is required'.
[error] 253-253: TestEachWithOptions/invalid_array_-_wrong_type: Error message mismatch. Expected: 'element at index 1: value must be a string', Actual: 'value must be a string'.
🔇 Additional comments (5)
validator/validator.go (1)
56-58: Enhanced error handling logic is well implemented.The conditional check for empty validator messages is a good improvement. It returns the original error when no custom message is provided, allowing for more detailed error information from the validator function while still supporting custom messages when specified.
validator/range_test.go (2)
259-384: Comprehensive validation structure for product variants.The validation options are well-structured with appropriate constraints for product variant fields. Good use of nested validators and proper error messages throughout.
386-474: Test cases cover important validation scenarios.The test cases effectively verify the validation rules with appropriate failure and success cases. The error checking logic is thorough, testing both presence and content of error messages.
validator/range.go (2)
9-31: Good enhancement to support multiple types in MinLength.The implementation now properly handles strings, slices, and arrays using reflection, with appropriate error messages for each case. The nil check is also a good addition.
33-55: Good enhancement to support multiple types in MaxLength.Similar to MinLength, the implementation now properly handles strings, slices, and arrays using reflection, with appropriate error messages for each case. The nil check is also a good addition.
| } | ||
| if err := Validate(nestedBody, options); err != nil { | ||
| return fmt.Errorf("element at index %d: %v", i, err) | ||
| return err |
There was a problem hiding this comment.
This change breaks existing tests.
Removing the element index from the error message breaks the tests in TestEachWithOptions that expect errors to include the index (see pipeline failures).
- return err
+ return fmt.Errorf("element at index %d: %v", i, err)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return err | |
| return fmt.Errorf("element at index %d: %v", i, err) |
1 participant
Summary by CodeRabbit
New Features
Tests