refactor: extract duplicate logic from observability filters and hooks into reusable utils#495
Merged
akila-i merged 7 commits intoopenchoreo:mainfrom Apr 7, 2026
Merged
Conversation
akila-i
requested review from
jhivandb,
kaviththiranga and
sameerajayasoma
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughRefactors observability plugin to centralize debounced search and environment auto-selection via two new hooks, removes local time-range utilities/constants, and updates components and hooks to import shared Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TextField as SearchInput
participant Hook as useDebouncedSearch
participant Component as ParentComponent
User->>TextField: types characters
TextField->>Hook: onChange events
Hook->>Hook: update local searchInput
Hook-->>Hook: debounce timer (e.g., 500-1000ms)
Hook->>Component: onChange callback with debounced searchQuery
Component->>Component: call onFiltersChange({ searchQuery })
sequenceDiagram
participant Component as FilterHook/Component
participant AutoHook as useAutoSelectFirstEnvironment
participant Envs as Environments
participant URL as URLSearchParams/setSearchParams
Component->>AutoHook: mount or searchParams change (environments, searchParams)
AutoHook->>Envs: read environments list
AutoHook->>URL: read current env param
alt env missing or not matching canonical
AutoHook->>URL: set env to Envs[0].name (setSearchParams with replace:true)
else env matches but not canonical name
AutoHook->>URL: rewrite env to canonical name (replace:true)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Signed-off-by: Akila-I <akila.99g@gmail.com>
Signed-off-by: Akila-I <akila.99g@gmail.com>
Signed-off-by: Akila-I <akila.99g@gmail.com>
…mary Signed-off-by: Akila-I <akila.99g@gmail.com>
…ve duplicates Signed-off-by: Akila-I <akila.99g@gmail.com>
…r components Signed-off-by: Akila-I <akila.99g@gmail.com>
akila-i
force-pushed
the
obs-refactor-duplicates
branch
from
cd5d72b to
4e380ed
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
plugins/openchoreo-observability/src/hooks/useUrlFiltersForAlerts.ts (1)
37-41: Normalize severity tokens before validation.Whitespace around comma-separated severity values is currently treated as invalid. Trimming before validation makes URL parsing more robust.
Suggested refactor
const severity = severityParam ? severityParam .split(',') + .map(s => s.trim()) .filter( s => Boolean(s) && (ALERT_SEVERITIES as readonly string[]).includes(s), ) : [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/openchoreo-observability/src/hooks/useUrlFiltersForAlerts.ts` around lines 37 - 41, The filter is rejecting values with surrounding whitespace; in useUrlFiltersForAlerts.ts update the split/filter pipeline to trim each token before validation so ALERT_SEVERITIES checks the normalized value (e.g., use s.trim() in the predicate and, if tokens are propagated later, map to the trimmed value) — change the lambda that uses (ALERT_SEVERITIES as readonly string[]).includes(s) to perform includes(s.trim()) and ensure downstream code uses the trimmed token.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@plugins/openchoreo-observability/src/components/Traces/ObservabilityTracesPage.test.tsx`:
- Around line 14-17: The mock implementation of calculateTimeRange uses new
Date().toISOString(), making tests time-dependent; update the calculateTimeRange
mock in ObservabilityTracesPage.test.tsx to return deterministic, fixed ISO
timestamps with a non-zero interval (e.g., a fixed start and a later end) so
startTime/endTime wiring is stable and reproducible during tests.
In `@plugins/openchoreo-observability/src/hooks/useAutoSelectFirstEnvironment.ts`:
- Around line 19-23: The current isValid check only accepts exact envParam ===
e.name, causing URLs that matched by displayName or case-insensitive name (as
handled in useUrlFilters) to be treated invalid and rewritten to
environments[0]; update the validation and rewrite logic in
useAutoSelectFirstEnvironment (the isValid variable that reads envParam and the
branch that creates newParams and calls setSearchParams) to consider a match
when envParam equals e.name, equals e.displayName, or equals e.name
case-insensitively, and if a match is found set the env param to that matched
environment's canonical e.name (instead of forcing environments[0].name); keep
falling back to environments[0].name only when no such match exists.
---
Nitpick comments:
In `@plugins/openchoreo-observability/src/hooks/useUrlFiltersForAlerts.ts`:
- Around line 37-41: The filter is rejecting values with surrounding whitespace;
in useUrlFiltersForAlerts.ts update the split/filter pipeline to trim each token
before validation so ALERT_SEVERITIES checks the normalized value (e.g., use
s.trim() in the predicate and, if tokens are propagated later, map to the
trimmed value) — change the lambda that uses (ALERT_SEVERITIES as readonly
string[]).includes(s) to perform includes(s.trim()) and ensure downstream code
uses the trimmed token.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fba7034e-305a-45a4-acd1-3db497e69bc1
📒 Files selected for processing (25)
plugins/openchoreo-observability/src/components/Alerts/AlertsFilter.tsxplugins/openchoreo-observability/src/components/Alerts/types.tsplugins/openchoreo-observability/src/components/Incidents/IncidentsFilter.tsxplugins/openchoreo-observability/src/components/Incidents/types.tsplugins/openchoreo-observability/src/components/RCA/RCAFilters.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/LogsFilter.tsxplugins/openchoreo-observability/src/components/RuntimeLogs/types.tsplugins/openchoreo-observability/src/components/RuntimeLogs/utils.tsplugins/openchoreo-observability/src/components/Traces/ObservabilityTracesPage.test.tsxplugins/openchoreo-observability/src/components/Traces/ObservabilityTracesPage.tsxplugins/openchoreo-observability/src/components/Traces/TracesFilters.tsxplugins/openchoreo-observability/src/components/Traces/utils.test.tsplugins/openchoreo-observability/src/components/Traces/utils.tsplugins/openchoreo-observability/src/hooks/useAutoSelectFirstEnvironment.tsplugins/openchoreo-observability/src/hooks/useComponentAlerts.tsplugins/openchoreo-observability/src/hooks/useDebouncedSearch.tsplugins/openchoreo-observability/src/hooks/useMetrics.tsplugins/openchoreo-observability/src/hooks/useProjectIncidents.tsplugins/openchoreo-observability/src/hooks/useProjectRuntimeLogs.tsplugins/openchoreo-observability/src/hooks/useRuntimeLogs.tsplugins/openchoreo-observability/src/hooks/useTraces.tsplugins/openchoreo-observability/src/hooks/useUrlFilters.tsplugins/openchoreo-observability/src/hooks/useUrlFiltersForAlerts.tsplugins/openchoreo-observability/src/hooks/useUrlFiltersForIncidents.tsplugins/openchoreo-observability/src/hooks/useUrlFiltersForRuntimeLogs.ts
💤 Files with no reviewable changes (5)
- plugins/openchoreo-observability/src/components/RuntimeLogs/types.ts
- plugins/openchoreo-observability/src/components/Alerts/types.ts
- plugins/openchoreo-observability/src/components/Incidents/types.ts
- plugins/openchoreo-observability/src/components/RuntimeLogs/utils.ts
- plugins/openchoreo-observability/src/components/Traces/utils.ts
…nment selection logic Signed-off-by: Akila-I <akila.99g@gmail.com>
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Contributor
|
@akila-i In many obs related views we use the same set of filters right ? Like Environment, Time Range and so on. Can we consider adding reusable UI components for these filters as well ? |
stefinie123
approved these changes
Apr 7, 2026
Contributor
Author
Ack. Will do that refactoring in a separate PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This pull request refactors and unifies the time range filter logic and debounced search handling across the observability plugin's filter components. It removes duplicated code, centralizes shared constants and utilities, and introduces a new hook for auto-selecting the first environment when none is selected. The changes also move the
calculateTimeRangeutility to a shared package for broader reuse.Key changes:
Refactoring and Code Reuse
ALERTS_TIME_RANGE_OPTIONS,INCIDENTS_TIME_RANGE_OPTIONS, etc.) with a single sharedTIME_RANGE_OPTIONSimported from../../types, ensuring consistency across all filter components. [1] [2] [3] [4] [5]calculateTimeRangefunctions from local utility files and imports it from the shared@openchoreo/backstage-plugin-reactpackage, both in implementation and tests. [1] [2] [3] [4] [5]Improved Debounced Search
useDebounceoruseEffect/setTimeout) in all filter components with a unified custom hook,useDebouncedSearch, simplifying code and standardizing behavior. [1] [2] [3] [4] [5]Type and Import Cleanups
New Utility Hook
useAutoSelectFirstEnvironment, which ensures that if no valid environment is selected in the URL query parameters, the first available environment is automatically selected. This helps maintain valid state across all URL filter hooks.Goals
Approach
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
New Features
Refactor
Tests