-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: Relative date filter #2736
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
base: alpha
Are you sure you want to change the base?
feat: Relative date filter #2736
Conversation
I will reformat the title to use the proper commit message syntax. |
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. |
Uffizzi Ephemeral Environment
|
Uffizzi Ephemeral Environment
|
📝 Walkthrough""" WalkthroughThe changes introduce support for saving filters with relative date values. The UI now presents a "Relative dates" checkbox when filters include date comparisons, and this option is passed through the save process. When enabled, date filters are stored as relative offsets instead of absolute dates. Path generation logic now resolves relative dates back to absolute dates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BrowserFilter
participant Browser
participant Preferences
User->>BrowserFilter: Initiate save filter (with/without "Relative dates")
BrowserFilter->>Browser: onSaveFilter(filters, name, relativeDates)
Browser->>Browser: saveFilters(filters, name, relativeDates)
alt relativeDates is true
Browser->>Browser: Convert absolute dates to relative offsets
end
Browser->>Preferences: Save filters
sequenceDiagram
participant App
participant generatePath
App->>generatePath: Request path with filters (may include RelativeDate)
generatePath->>generatePath: Convert RelativeDate to absolute ISO date
generatePath-->>App: Return path with updated filters
Assessment against linked issues
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. 🔧 ESLint
npm warn config production Use Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🪛 Biome (1.9.4)src/dashboard/Data/Browser/Browser.react.js[error] 1086-1086: Avoid the delete operator which can impact performance. Unsafe fix: Use an undefined assignment instead. (lint/performance/noDelete) 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/BrowserFilter/BrowserFilter.react.js
(5 hunks)src/dashboard/Data/Browser/Browser.react.js
(2 hunks)src/lib/generatePath.js
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/components/BrowserFilter/BrowserFilter.react.js (2)
src/components/Field/Field.react.js (1)
Field
(12-34)src/components/Label/Label.react.js (1)
Label
(13-24)
🪛 Biome (1.9.4)
src/dashboard/Data/Browser/Browser.react.js
[error] 1083-1083: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Docker linux/amd64
🔇 Additional comments (5)
src/components/BrowserFilter/BrowserFilter.react.js (5)
20-20
: LGTM - Clean import of the Checkbox componentThe Checkbox component is correctly imported for the new UI feature.
36-36
: LGTM - Good state initializationThe relativeDates state is properly initialized to false.
119-119
: LGTM - Correctly passing the relativeDates parameterThe
save
method now passes the relativeDates state to the onSaveFilter callback.
143-143
: Effective detection of date filtersGood approach to conditionally show the relative dates option only when relevant date filters exist.
187-211
: Well-structured UI for the relative dates optionThe implementation correctly:
- Wraps the fields in a React Fragment
- Only shows the relative dates checkbox when date filters exist
- Properly binds to state with appropriate handlers
The code is clean and follows the established patterns in the codebase.
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
🧹 Nitpick comments (1)
src/dashboard/Data/Browser/Browser.react.js (1)
1082-1083
: Consider adding ISO 8601 duration format as an alternative representationCurrently, the relative date is stored as a simple number of seconds. For better readability and maintainability, consider supporting ISO 8601 duration format (e.g., "P1DT2H30M" for 1 day, 2 hours, and 30 minutes) as an alternative representation for human-readable filters.
This would make saved filters more intuitive when debugging or when manually editing saved preferences.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1083-1083: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/dashboard/Data/Browser/Browser.react.js
(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/dashboard/Data/Browser/Browser.react.js
[error] 1083-1083: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Docker linux/amd64
🔇 Additional comments (1)
src/dashboard/Data/Browser/Browser.react.js (1)
1073-1077
:✅ Verification successful
Verify handling of past dates in relative filter calculations
The code correctly calculates the time difference between the filter date and current time. However, for dates in the past, this will result in a negative value stored in
compareTo.value
. Make sure the code that consumes this value (ingeneratePath.js
) properly handles negative relative date values.Run the following script to check how negative relative date values are handled:
🏁 Script executed:
#!/bin/bash # Search for where RelativeDate type is processed to ensure negative values are handled correctly # Look for RelativeDate handling in the generatePath.js file grep -A 20 "RelativeDate" src/lib/generatePath.js # Look for any other places where RelativeDate might be processed rg "RelativeDate" --type jsLength of output: 872
Negative RelativeDate offsets are handled correctly
Insrc/lib/generatePath.js
, the code appliesfilter.compareTo.value
(in seconds) directly toDate.getTime()
, so negative values naturally yield past dates as expected. No additional handling is required.
Uffizzi Ephemeral Environment
|
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.
The times seem to be correct now, but I found that:
- A selected filter with relative dates is not highlighted anymore in the nav bar.
- The "Relative dates" checkbox is persistent during the dashboard session. For example, save a filter with "Relative dates" selected, then click on the classname, then create a new filter and the save dialog shows "Relative dates" still selected. Checkbox selection should not be persistent.
New Pull Request Checklist
Issue Description
Closes: #2658
Approach
TODOs before merging
Summary by CodeRabbit