-
Notifications
You must be signed in to change notification settings - Fork 89
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: validation support filters #549
Conversation
WalkthroughThe recent updates introduce a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ContextRequest
participant Validation
participant Validator
Client->>ContextRequest: Validate(data, rules, filters)
ContextRequest->>Validation: Make(data, rules, filters)
Validation->>Validator: ApplyFilters(data, filters)
Validator->>Validation: Validate(data, rules)
Validation-->>ContextRequest: Validator
ContextRequest-->>Client: Validation result
Assessment against linked issues
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #549 +/- ##
==========================================
+ Coverage 69.91% 70.02% +0.10%
==========================================
Files 180 180
Lines 11053 11106 +53
==========================================
+ Hits 7728 7777 +49
- Misses 2752 2754 +2
- Partials 573 575 +2 ☔ View full report in Codecov by Sentry. |
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: 0
Outside diff range, codebase verification and nitpick comments (1)
validation/validation_test.go (1)
23-23
: Add a brief comment explaining thefilters
field.To improve readability, add a comment explaining what the
filters
field is used for in the test cases.+ // filters specify data manipulation rules before validation. filters map[string]string
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
mocks/validation/Validation.go
is excluded by!mocks/**
Files selected for processing (6)
- contracts/validation/validation.go (1 hunks)
- validation/errors_test.go (10 hunks)
- validation/validation.go (6 hunks)
- validation/validation_test.go (130 hunks)
- validation/validator.go (1 hunks)
- validation/validator_test.go (13 hunks)
Files not summarized due to errors (1)
- validation/validation_test.go: Error: Message exceeds token limit
Additional comments not posted (58)
contracts/validation/validation.go (1)
7-7
: Verify the impact of the newfilters
parameter in theMake
method.Adding the
filters
parameter to theMake
method changes the interface and requires all implementations to support this new parameter. Ensure that all implementations of theValidation
interface are updated accordingly.validation/validator.go (1)
43-43
: LGTM! Proper error handling in theBind
method.Changing the assignment operator from
:=
to=
ensures that theerr
variable is not re-declared within the method scope, which is crucial for accurate error handling.validation/errors_test.go (19)
17-17
: LGTM! Addedfilters
field to test cases.The addition of the
filters
field in test data structs ensures that the new filter functionality is tested.
25-25
: LGTM! Updated test cases to includefilters
.The test case ensures that the filter
trim
is correctly applied to the data.
31-31
: LGTM! Updated test cases to includefilters
.The test case ensures that the filter
trim
is correctly applied and validation errors are checked.
38-38
: LGTM! Updated test cases to includefilters
.The test case ensures that the filter
trim
is correctly applied and custom messages are handled.
48-48
: LGTM! Updated test cases to includefilters
.The test case ensures that the filter
trim
is correctly applied and custom attributes are handled.
58-58
: LGTM! Updated test cases to includefilters
.The test case ensures that the filter
trim
is correctly applied and both custom messages and attributes are handled.
72-72
: LGTM! Verified the newfilters
parameter in theMake
method call.The test case correctly includes the new
filters
parameter when calling theMake
method.
88-88
: LGTM! Addedfilters
field to test cases.The addition of the
filters
field in test data structs ensures that the new filter functionality is tested.
96-96
: LGTM! Updated test cases to includefilters
.The test case ensures that the filters
trim
are correctly applied to the data.
102-102
: LGTM! Updated test cases to includefilters
.The test case ensures that the filters
trim
are correctly applied and validation errors are checked.
113-113
: LGTM! Verified the newfilters
parameter in theMake
method call.The test case correctly includes the new
filters
parameter when calling theMake
method.
131-131
: LGTM! Addedfilters
field to test cases.The addition of the
filters
field in test data structs ensures that the new filter functionality is tested.
138-138
: LGTM! Updated test cases to includefilters
.The test case ensures that the filters
trim
are correctly applied to the data.
145-145
: LGTM! Updated test cases to includefilters
.The test case ensures that the filters
trim
are correctly applied and validation errors are checked.
158-158
: LGTM! Verified the newfilters
parameter in theMake
method call.The test case correctly includes the new
filters
parameter when calling theMake
method.
173-173
: LGTM! Addedfilters
field to test cases.The addition of the
filters
field in test data structs ensures that the new filter functionality is tested.
180-180
: LGTM! Updated test cases to includefilters
.The test case ensures that the filters
trim
are correctly applied to the data.
186-186
: LGTM! Updated test cases to includefilters
.The test case ensures that the filters
trim
are correctly applied and validation errors are checked.
196-196
: LGTM! Verified the newfilters
parameter in theMake
method call.The test case correctly includes the new
filters
parameter when calling theMake
method.validation/validation.go (4)
25-25
: LGTM! UpdatedMake
method to includefilters
.The
Make
method now includes afilters
parameter, enhancing the validation framework's functionality.
54-54
: LGTM! Verified the newfilters
parameter in theMake
method call.The method correctly includes the new
filters
parameter when callingGenerateOptions
.
75-82
: LGTM! Added logic to check for duplicate filter names.The logic ensures that duplicate filter names are not added, enhancing the robustness of the validation framework.
327-378
: LGTM! Added method to get existing filter names.The method
existFilterNames
provides a list of existing filter names, which is used to check for duplicates.validation/validator_test.go (22)
28-32
: Addition offilters
parameter in the test structure.The inclusion of the
filters
map in the test structure is appropriate for testing the new filter functionality.
35-41
: Test case for trimming whitespace from a map value.The test case correctly verifies the trimming of whitespace using the new
filters
parameter.
44-50
: Test case for integer validation from a map value.The test case correctly verifies the integer validation using the new
filters
parameter.
53-59
: Test case for casting a string to an integer without filters.The test case correctly verifies the casting of a string to an integer without applying any filters.
62-68
: Test case for handling validation errors with filters.The test case correctly verifies the handling of validation errors and the application of filters.
72-78
: Test case for trimming whitespace from an uppercase map key.The test case correctly verifies the trimming of whitespace from an uppercase map key using the new
filters
parameter.
Line range hint
83-93
:
Test case for trimming whitespace from a struct field.The test case correctly verifies the trimming of whitespace from a struct field using the new
filters
parameter.
104-111
: Test case for handling lowercase struct field.The test case correctly verifies the handling of a lowercase struct field and the application of filters.
123-131
: Test case for handling nested struct fields.The test case correctly verifies the handling of nested struct fields and the application of filters.
139-147
: Test case for trimming whitespace from a GET request parameter.The test case correctly verifies the trimming of whitespace from a GET request parameter using the new
filters
parameter.
162-163
: Potential issue with filter type.The filter type for the integer parameter should be reviewed. The
trim
filter might not be appropriate for an integer.Ensure that the
trim
filter is intended for this test case. If not, update the filter to an appropriate one for integers.
171-180
: Test case for trimming whitespace from a POST request parameter.The test case correctly verifies the trimming of whitespace from a POST request parameter using the new
filters
parameter.
194-195
: Test case for POST request with file upload.The test case correctly verifies the handling of a POST request with a file upload and the application of filters.
Line range hint
210-215
:
Integration offilters
parameter in the validation logic.The integration of the
filters
parameter in the validation logic is appropriate and ensures the filters are applied during the test execution.
228-229
: Addition offilters
parameter in the test structure.The inclusion of the
filters
map in the test structure is appropriate for testing the new filter functionality.
235-235
: Test case for validation failure without filters.The test case correctly verifies the validation failure without applying any filters.
241-241
: Test case for validation failure with filters.The test case correctly verifies the validation failure with filters applied.
251-254
: Integration offilters
parameter in the validation logic.The integration of the
filters
parameter in the validation logic is appropriate and ensures the filters are applied during the test execution.
282-283
: Addition offilters
parameter in the test structure.The inclusion of the
filters
map in the test structure is appropriate for testing the new filter functionality.
335-335
: Test case for validation without casting data.The test case correctly verifies the validation without casting data and ensures the filters are applied.
403-403
: Test case for validation with casting data.The test case correctly verifies the validation with casting data and ensures the filters are applied.
428-431
: Integration offilters
parameter in the validation logic.The integration of the
filters
parameter in the validation logic is appropriate and ensures the filters are applied during the test execution.validation/validation_test.go (11)
33-35
: Ensure comprehensive filter scenarios.The test case for
map[string]any
data type includes a filter for trimming whitespace. Ensure that other common filters (e.g., lowercase, uppercase) are also tested in different scenarios.Would you like to add more test cases to cover additional filters?
41-43
: Ensure comprehensive filter scenarios for struct data.The test case for struct data includes a filter for trimming whitespace. Ensure that other common filters (e.g., lowercase, uppercase) are also tested in different scenarios.
Would you like to add more test cases to cover additional filters for struct data?
49-51
: Check error handling for invalid filter usage.The test case for invalid data types includes a filter. Ensure that the error handling for invalid filter usage is tested.
Would you like to add test cases to cover invalid filter usage scenarios?
58-58
: Check error handling for empty data with filters.The test case for empty data includes a filter. Ensure that the error handling for empty data with filters is tested.
Would you like to add test cases to cover empty data scenarios with different filters?
65-65
: Check error handling for empty rules with filters.The test case for empty rules includes filters. Ensure that the error handling for empty rules with filters is tested.
Would you like to add test cases to cover empty rules scenarios with different filters?
70-72
: Check error handling forPrepareForValidation
with filters.The test case for
PrepareForValidation
includes filters. Ensure that the error handling forPrepareForValidation
with filters is tested.Would you like to add test cases to cover
PrepareForValidation
scenarios with different filters?
82-84
: Ensure comprehensive filter scenarios withPrepareForValidation
.The test case for
PrepareForValidation
includes a filter. Ensure that other common filters (e.g., lowercase, uppercase) are also tested withPrepareForValidation
.Would you like to add more test cases to cover additional filters with
PrepareForValidation
?
99-101
: Ensure comprehensive filter scenarios with multiple options.The test case for multiple options includes filters. Ensure that other common filters (e.g., lowercase, uppercase) are also tested with multiple options.
Would you like to add more test cases to cover additional filters with multiple options?
126-126
: Ensure comprehensive filter scenarios for struct data withPrepareForValidation
.The test case for struct data with
PrepareForValidation
includes a filter. Ensure that other common filters (e.g., lowercase, uppercase) are also tested for struct data withPrepareForValidation
.Would you like to add more test cases to cover additional filters for struct data with
PrepareForValidation
?
143-143
: Ensure comprehensive filter scenarios for struct data with multiple options.The test case for struct data with multiple options includes filters. Ensure that other common filters (e.g., lowercase, uppercase) are also tested for struct data with multiple options.
Would you like to add more test cases to cover additional filters for struct data with multiple options?
169-172
: Ensure consistent validation of expected data.The test cases validate expected data after applying filters. Ensure that all filters are correctly applied and validated.
Would you like to add assertions to validate the application of all filters?
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
mocks/http/ContextRequest.go
is excluded by!mocks/**
Files selected for processing (1)
- contracts/http/request.go (2 hunks)
Additional comments not posted (2)
contracts/http/request.go (2)
84-84
: LGTM! The addition of thefilters
parameter is consistent with the PR objective.The
Validate
method now includes afilters
parameter, which aligns with the goal of enhancing data validation processes.
95-96
: LGTM! The reordering of theFilters
method is minor and logical.The
Filters
method is now placed betweenRules
andMessages
, which aligns with organizing the methods in a logical sequence.
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.
Great 👍
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- contracts/http/request.go (1 hunks)
- validation/errors_test.go (9 hunks)
- validation/validation.go (4 hunks)
- validation/validation_test.go (6 hunks)
- validation/validator.go (1 hunks)
- validation/validator_test.go (13 hunks)
Files skipped from review as they are similar to previous changes (5)
- contracts/http/request.go
- validation/errors_test.go
- validation/validation.go
- validation/validator.go
- validation/validator_test.go
Additional comments not posted (16)
validation/validation_test.go (16)
31-36
: Validate the inclusion of filters in the test case.The
TestMake
test case has been updated to include filters. Ensure that the filters are correctly applied and tested.
41-46
: Validate the inclusion of filters in the test case.The
TestMake
test case has been updated to include filters. Ensure that the filters are correctly applied and tested.
52-57
: Validate the inclusion of filters in the test case.The
TestMake
test case has been updated to include filters. Ensure that the filters are correctly applied and tested.
63-65
: Validate the inclusion of filters in the test case.The
TestMake
test case has been updated to include filters. Ensure that the filters are correctly applied and tested.
76-79
: Validate the inclusion of filters in the test case.The
TestMake
test case has been updated to include filters. Ensure that the filters are correctly applied and tested.
88-91
: Validate the inclusion of filters in the test case.The
TestMake
test case has been updated to include filters. Ensure that the filters are correctly applied and tested.
105-108
: Validate the inclusion of filters in the test case.The
TestMake
test case has been updated to include filters. Ensure that the filters are correctly applied and tested.
133-136
: Validate the inclusion of filters in the test case.The
TestMake
test case has been updated to include filters. Ensure that the filters are correctly applied and tested.
150-153
: Validate the inclusion of filters in the test case.The
TestMake
test case has been updated to include filters. Ensure that the filters are correctly applied and tested.
Line range hint
378-384
:
Validate the correctness of theTestAddFilter
test case.The
TestAddFilter
test case has been added to test the addition of filters. Ensure the test case correctly verifies the filter functionality.
Line range hint
386-396
:
Validate the correctness of theTestAddFilters
test case.The
TestAddFilters
test case has been added to test the addition of multiple filters. Ensure the test case correctly verifies the filter functionality.
Line range hint
398-432
:
Validate the correctness of theTestFilters
test case.The
TestFilters
test case has been added to test the application of filters. Ensure the test case correctly verifies the filter functionality.
Line range hint
579-624
:
Validate the correctness of theTestCustomRule
test case.The
TestCustomRule
test case has been added to test the addition of custom rules. Ensure the test case correctly verifies the custom rule functionality.
Line range hint
626-646
:
Validate the correctness of theUppercase
struct.The
Uppercase
struct has been added to define a custom validation rule. Ensure the struct correctly implements the custom rule functionality.
Line range hint
648-668
:
Validate the correctness of theLowercase
struct.The
Lowercase
struct has been added to define a custom validation rule. Ensure the struct correctly implements the custom rule functionality.
Line range hint
670-690
:
Validate the correctness of theDuplicate
struct.The
Duplicate
struct has been added to define a custom validation rule. Ensure the struct correctly implements the custom rule functionality.
📑 Description
Closes goravel/goravel#416
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✅ Checks