chore(weave): add date range filter? #3887
Conversation
|
New, updated, and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@mui/[email protected], npm/@mui/[email protected] |
|
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat is a license policy violation?This package is not allowed per your license policy. Review the package's license to ensure compliance. Find a package that does not violate your license policy or adjust your policy to allow this package's license. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
WalkthroughThis pull request includes a migration from the standard Material-UI date picker to its Pro version by updating dependency references and imports. New components for handling date range selections—such as a styled date range picker, a date range filter, and a display component—are introduced. Additionally, operator options and handling logic for date range filtering (using the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant FRow as FilterRow
participant SVal as SelectValue
participant DRP as DateRangePicker
participant DDisp as DateRangeDisplay
participant OpConv as OperatorConverter
U->>FRow: Selects "(date): range" operator
FRow->>FRow: Initialize filter value with default JSON range
FRow->>SVal: Sends updated filter value
SVal->>DRP: Renders DateRangePicker (for "(date): range")
U->>DRP: Picks start and end dates
DRP->>SVal: Calls onSetValue with updated date JSON
SVal->>DDisp: Passes updated value for display
DDisp->>OpConv: Data used in building query filters
Poem
✨ 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
Documentation and Community
|
![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 (2)
weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/tabularListViews/operators.ts (2)
151-173: Consider adding date validation in the range filter.While the implementation is solid, it might be beneficial to add validation for the date values. For example, checking if the end date is greater than or equal to the start date, and handling potential invalid date inputs.
try { // Parse the JSON string containing start and end dates const {start, end} = JSON.parse(item.value); + // Validate dates if both are present + if (start && end) { + const startDate = new Date(start); + const endDate = new Date(end); + + // Check if dates are valid + if (isNaN(startDate.getTime()) || isNaN(endDate.getTime())) { + console.error('Invalid date format in range filter'); + return null; + } + + // Check if end date is not before start date + if (endDate < startDate) { + console.error('End date is before start date in range filter'); + return null; + } + } // If both start and end are specified, use $and to combine them if (start && end) {
194-199: Consider adding more detailed error messages.The current error logging only includes the error object itself. Adding more context about the problematic value would make debugging easier.
} catch (e) { - console.error('Error parsing date range value:', e); + console.error('Error parsing date range value:', item.value, e); return null; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
weave-js/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
weave-js/package.json(1 hunks)weave-js/src/components/PagePanelComponents/Home/Browse3/StyledDateRangePicker.tsx(1 hunks)weave-js/src/components/PagePanelComponents/Home/Browse3/StyledDateTimePicker.tsx(1 hunks)weave-js/src/components/PagePanelComponents/Home/Browse3/filters/DateRangePicker.tsx(1 hunks)weave-js/src/components/PagePanelComponents/Home/Browse3/filters/FilterPanel.tsx(1 hunks)weave-js/src/components/PagePanelComponents/Home/Browse3/filters/FilterRow.tsx(1 hunks)weave-js/src/components/PagePanelComponents/Home/Browse3/filters/FilterTagItem.tsx(2 hunks)weave-js/src/components/PagePanelComponents/Home/Browse3/filters/SelectValue.tsx(3 hunks)weave-js/src/components/PagePanelComponents/Home/Browse3/filters/common.ts(2 hunks)weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/tabularListViews/operators.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.{js,jsx,ts,tsx}`: Focus on architectural and logical i...
**/*.{js,jsx,ts,tsx}: Focus on architectural and logical issues rather than style (assuming ESLint is in place).
Flag potential memory leaks and performance bottlenecks.
Check for proper error handling and async/await usage.
Avoid strict enforcement of try/catch blocks - accept Promise chains, early returns, and other clear error handling patterns. These are acceptable as long as they maintain clarity and predictability.
Ensure proper type usage in TypeScript files.
Look for security vulnerabilities in data handling.
Don't comment on formatting if prettier is configured.
Verify proper React hooks usage and component lifecycle.
Check for proper state management patterns.
weave-js/src/components/PagePanelComponents/Home/Browse3/StyledDateRangePicker.tsxweave-js/src/components/PagePanelComponents/Home/Browse3/filters/FilterPanel.tsxweave-js/src/components/PagePanelComponents/Home/Browse3/filters/FilterRow.tsxweave-js/src/components/PagePanelComponents/Home/Browse3/filters/FilterTagItem.tsxweave-js/src/components/PagePanelComponents/Home/Browse3/StyledDateTimePicker.tsxweave-js/src/components/PagePanelComponents/Home/Browse3/filters/common.tsweave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/tabularListViews/operators.tsweave-js/src/components/PagePanelComponents/Home/Browse3/filters/DateRangePicker.tsxweave-js/src/components/PagePanelComponents/Home/Browse3/filters/SelectValue.tsx
`**/*.{yaml,yml,json,tf}`: Check for security best practices...
**/*.{yaml,yml,json,tf}: Check for security best practices.
Verify environment-specific configurations.
Look for hardcoded credentials or sensitive data.
Ensure proper resource limits and requests.
Verify proper versioning of dependencies.
Check for infrastructure best practices.
weave-js/package.json
⏰ Context from checks skipped due to timeout of 90000ms (110)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: WeaveJS Lint and Compile
- GitHub Check: Trace nox tests (3, 13, llamaindex)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, scorers)
- GitHub Check: Trace nox tests (3, 12, llamaindex)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, scorers)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, scorers)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 9, scorers)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: WeaveJS Lint and Compile
- GitHub Check: Trace nox tests (3, 13, llamaindex)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, scorers)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, scorers)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, scorers)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 9, scorers)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: WeaveJS Lint and Compile
- GitHub Check: Trace nox tests (3, 13, llamaindex)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, scorers)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, scorers)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, scorers)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 9, scorers)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: WeaveJS Lint and Compile
- GitHub Check: Trace nox tests (3, 13, llamaindex)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, scorers)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, scorers)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, scorers)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 9, scorers)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: WeaveJS Lint and Compile
- GitHub Check: Trace nox tests (3, 13, llamaindex)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, scorers)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, scorers)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, scorers)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 9, scorers)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: WeaveJS Lint and Compile
- GitHub Check: Trace nox tests (3, 13, llamaindex)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, scorers)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, scorers)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, scorers)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 9, scorers)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: WeaveJS Lint and Compile
- GitHub Check: Trace nox tests (3, 13, llamaindex)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, scorers)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, scorers)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, scorers)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 9, scorers)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: WeaveJS Lint and Compile
- GitHub Check: Trace nox tests (3, 13, llamaindex)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, scorers)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, scorers)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, scorers)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 9, scorers)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: WeaveJS Lint and Compile
- GitHub Check: Trace nox tests (3, 13, llamaindex)
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, scorers)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, scorers)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, scorers)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 9, scorers)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: WeaveJS Lint and Compile
- GitHub Check: Trace nox tests (3, 13, trace)
- GitHub Check: Trace nox tests (3, 12, scorers)
- GitHub Check: Trace nox tests (3, 12, trace)
- GitHub Check: Trace nox tests (3, 11, scorers)
- GitHub Check: Trace nox tests (3, 11, trace)
- GitHub Check: Trace nox tests (3, 10, scorers)
- GitHub Check: Trace nox tests (3, 10, trace)
- GitHub Check: Trace nox tests (3, 9, scorers)
🔇 Additional comments (17)
weave-js/src/components/PagePanelComponents/Home/Browse3/filters/FilterPanel.tsx (1)
6-7: Package upgrade looks correct.The imports have been appropriately updated to use the Pro version of the MUI date pickers. This change aligns with the package.json update and enables the new date range filtering capabilities.
weave-js/package.json (1)
50-50:✅ Verification successful
Verify dependency compatibility with other components.
The upgrade from
@mui/x-date-pickersv6.16.0 to@mui/x-date-pickers-prov7.28.0 is a significant version jump that includes the Pro features needed for date range functionality. Make sure all components using these date pickers have been updated to be compatible with v7.x.
🏁 Script executed:
#!/bin/bash # Search for all imports of date picker components to verify they're updated rg -A 1 $'from \'@mui/x-date-pickers' --type ts --type tsxLength of output: 89
I'll re-run the search command with explicit globs for TypeScript and TSX files to ensure we capture all relevant imports. This should help verify whether any components still import from
@mui/x-date-pickersinstead of the updated@mui/x-date-pickers-pro.
🏁 Script executed:
#!/bin/bash # Search for all imports of the outdated package in .ts and .tsx files rg "from\s+['\"]@mui\/x-date-pickers" --glob "*.ts" --glob "*.tsx" -n .Length of output: 188
I've adjusted the search command to fix the regex issue. Please re-run the following script to verify that no files still import from the outdated package (
@mui/x-date-pickers):Once you verify that no results appear, you can confidently conclude that all components have been updated to use
@mui/x-date-pickers-prov7.28.0. If any matches are found, please inspect those files for compatibility issues.Please verify the dependency update manually if the command returns any matches.
🏁 Script executed:
#!/bin/bash # Re-run search for imports from the outdated package in .ts and .tsx files using a corrected regex rg "from\\s+[\"']@mui/x-date-pickers" --glob "*.ts" --glob "*.tsx" -n .Length of output: 704
Dependency Upgrade Verified for Date Picker Components
The dependency in
weave-js/package.jsonhas been updated to use@mui/x-date-pickers-prov7.28.0, and our verification confirms that all date picker imports in the codebase now reference the updated package. For example, files such as:
weave-js/src/components/PagePanelComponents/Home/Browse3/StyledDateRangePicker.tsxweave-js/src/components/PagePanelComponents/Home/Browse3/StyledDateTimePicker.tsxweave-js/src/components/PagePanelComponents/Home/Browse3/filters/FilterPanel.tsxall correctly import components from
@mui/x-date-pickers-pro.Please ensure that the runtime behavior and Pro features (including date range functionality) work as expected after the upgrade.
weave-js/src/components/PagePanelComponents/Home/Browse3/filters/SelectValue.tsx (2)
19-19: LGTM: New import for DateRangePicker.This import is correctly added to support the new date range filtering capability.
55-57: Implementation of date range filter looks good.The conditional logic for handling the new date range operator is well-placed within the datetime field type section. The component passes the appropriate props to the DateRangePicker to manage state.
This implementation correctly uses JSON string handling for the date range values as shown in the DateRangePicker component implementation:
// From DateRangePicker.tsx const parsedValue = React.useMemo(() => { try { if (!value) { return {start: null, end: null}; } return JSON.parse(value); } catch (e) { return {start: null, end: null}; } }, [value]);weave-js/src/components/PagePanelComponents/Home/Browse3/StyledDateTimePicker.tsx (1)
2-2: Updated import path consistent with package change.The import has been correctly updated to use the Pro version of the date picker components, which aligns with the package.json changes.
weave-js/src/components/PagePanelComponents/Home/Browse3/filters/FilterTagItem.tsx (2)
23-23: Looks good: DateRangeDisplay import added.The new import for DateRangeDisplay from the DateRangePicker module is correctly implemented.
54-58: Clean implementation for date range display handling.The conditional logic for rendering different components based on the operator type is well-implemented. The code correctly distinguishes between the new date range operator and other date operators, rendering the appropriate component in each case.
weave-js/src/components/PagePanelComponents/Home/Browse3/StyledDateRangePicker.tsx (3)
1-7: Appropriate imports for the new styled component.The imports are properly organized, including the required dependencies from Material-UI, common styles, and utility functions.
8-11: Good variable naming for color constants.Using named constants for colors improves readability and maintainability. The use of hex to RGB conversion for transparency is a good practice.
12-45: Well-structured styled component with comprehensive styling.The styled component is well-implemented with:
- Proper wrapping of the DateRangePicker from MUI
- Comprehensive styling for different states (default, hover, focused)
- Appropriate adjustments for padding, margins, and font styles
- Good use of CSS selectors to target specific elements within the component
This follows best practices for creating styled components in Material-UI.
weave-js/src/components/PagePanelComponents/Home/Browse3/filters/FilterRow.tsx (1)
51-62: Good implementation of date range initialization.The refactoring of
onSelectOperatorto handle the date range operator is well done:
- Descriptive parameter name change from
operatortonewOperatorimproves clarity- Proper initialization of the value with a JSON string containing empty start/end dates when the date range operator is selected
- Maintaining the original value for other operators preserves existing functionality
This ensures a smooth transition when users select the date range filter.
weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/tabularListViews/operators.ts (1)
146-200: Comprehensive implementation of date range filtering logic.The implementation of the date range operator is well-structured and handles all necessary cases:
- Empty value check at the beginning is consistent with other operators
- JSON parsing is properly wrapped in a try-catch block to handle potential errors
- Clear handling of all possible combinations (both dates, start only, end only, neither)
- Appropriate query construction using
$and,$gt, and$notoperators for each case- Good error logging in the catch block
This implementation aligns well with the existing pattern for other operators while adding the necessary complexity for date ranges.
weave-js/src/components/PagePanelComponents/Home/Browse3/filters/common.ts (2)
153-157: Good addition of a date range filter operator.The new "between" operator with value "(date): range" fits well with the existing date filter operators. This addition will provide users with a more intuitive way to filter dates within a specific range.
259-263: Consistent implementation of the date range operator.The date range operator has been correctly added to the available options for datetime fields, maintaining consistency with how other operators are implemented. This ensures the new operator will be available when filtering date fields.
weave-js/src/components/PagePanelComponents/Home/Browse3/filters/DateRangePicker.tsx (3)
15-52: Well-implemented date range picker component with proper error handling.The DateRangePicker component is well structured with:
- Robust error handling for JSON parsing
- Proper use of React.useMemo to optimize performance
- Clean conversion between ISO strings and moment objects
- Clear prop interface
The component properly handles edge cases like empty values and parsing errors by defaulting to null values for both start and end dates.
31-40: Clean implementation of the date range change handler.The handleRangeChange function correctly:
- Extracts new start and end dates from the input array
- Converts moment objects to ISO strings with null handling
- Properly structures and stringifies the object before passing to onSetValue
This maintains a consistent data format regardless of the UI interaction.
54-70: Well-implemented date range display component.The DateRangeDisplay component:
- Properly handles empty values and parsing errors
- Uses a clean try/catch block for error handling
- Formats dates consistently with fallback text for undefined dates
- Returns null in error cases rather than throwing exceptions
The component provides a clean user display of the selected date range.
|
Preview this PR with FeatureBee: https://beta.wandb.ai/?betaVersion=335379b674e5f38b35d164553199e5c43c67f6e8 |
Description
Adds a between operator for dates (date range) which might be useful when we have multiple dates, rather than having a before and after. This could be auto-constructed when we detect before + after.
Maybe roll this out just to admin first
Testing
Summary by CodeRabbit