feat(pkg): add IsEmpty method to filters and update list events param… #2508
Conversation
📝 WalkthroughWalkthroughThe changes enhance input validation and filtering functionality. In the event service, the Changes
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 0
🧹 Nitpick comments (1)
openmeter/meterevent/service.go (1)
182-182: Enhanced validation logic for filter combinationsThe additional checks for
!p.Time.IsEmpty()and!p.IngestedAt.IsEmpty()improve the validation by ensuring that both filters contain valid, non-empty values if they are to be used together.Consider updating the error message to more accurately reflect the condition being checked, as it now allows both filters to be set if at least one is empty:
- errs = append(errs, errors.New("time and ingested_at cannot both be set")) + errs = append(errs, errors.New("time and ingested_at cannot both be set with non-empty values"))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
openmeter/meterevent/service.go(1 hunks)pkg/filter/filter.go(6 hunks)pkg/filter/filter_test.go(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
pkg/filter/filter_test.go (1)
pkg/filter/filter.go (5)
FilterString(32-47)FilterInteger(161-170)FilterFloat(270-279)FilterBoolean(481-483)FilterTime(378-385)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: CI
- GitHub Check: Analyze (go)
🔇 Additional comments (11)
pkg/filter/filter.go (6)
19-20: Well-designed interface extensionAdding the
IsEmpty()method to theFilterinterface is a clean way to enhance filtering capabilities across all implementation types.
115-118: Thorough implementation for FilterStringThe implementation correctly checks all possible filter fields in the FilterString struct to determine if the filter is empty.
236-239: Consistent implementation for FilterIntegerThe implementation follows the same pattern as FilterString, checking all fields to ensure consistency.
344-347: Consistent implementation for FilterFloatThe implementation properly checks all fields in the FilterFloat struct.
451-454: Proper implementation for FilterTimeThe method correctly identifies if a FilterTime instance has any configured filter criteria.
496-499: Simple implementation for FilterBooleanThe implementation is appropriately simple for FilterBoolean as it only has one field to check.
pkg/filter/filter_test.go (5)
1511-1632: Comprehensive tests for FilterString.IsEmptyThe test cases thoroughly cover all possible scenarios for the FilterString type, testing each field individually to ensure the IsEmpty method works correctly.
1634-1713: Thorough testing for FilterInteger.IsEmptyThe test cases comprehensively validate the IsEmpty functionality for FilterInteger type, covering empty filters and all possible field combinations.
1715-1794: Comprehensive tests for FilterFloat.IsEmptyAll relevant cases for FilterFloat are tested, maintaining consistency with the test approach for other filter types.
1796-1829: Complete test coverage for FilterBoolean.IsEmptyThe tests appropriately cover both true and false values for the FilterBoolean type's Eq field.
1831-1898: Well-structured tests for FilterTime.IsEmptyThe test cases provide good coverage for all possible field combinations in FilterTime, ensuring the IsEmpty method works as expected.
Summary by CodeRabbit
Bug Fixes
New Features
Tests