NETOBSERV-2872 feature views - #1595
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a view-preset feature system (types, presets, capability/URL/local-storage wiring, ViewSelector UI), updates config feature flags and column/panel defaults accordingly, switches Makefile/.mk port-cleanup from fuser to lsof, and reformats AGENTS.md with a new Views Feature section. ChangesView Presets Feature
Estimated code review effort: 4 (Complex) | ~50 minutes Backend Port-Kill Mechanism Update
Estimated code review effort: 1 (Trivial) | ~5 minutes AGENTS.md Documentation Reformatting
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Your configuration references 2 linked repositories, but your current plan allows 0. Analyzed ``, skipped 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 |
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #1595 +/- ##
===========================================
- Coverage 52.41% 40.83% -11.59%
===========================================
Files 241 48 -193
Lines 13086 3355 -9731
Branches 1704 0 -1704
===========================================
- Hits 6859 1370 -5489
+ Misses 5580 1811 -3769
+ Partials 647 174 -473
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
Makefile (1)
126-132: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSame trap string duplicated 4×.
The
lsof -ti tcp:9002 | xargs kill -9 2>/dev/null || truetrap is repeated verbatim instart,start-backend(this file), andstart-standalone/start-standalone-mockin.mk/standalone.mk. Consider aKILL_9002 := lsof -ti tcp:9002 | xargs kill -9 2>/dev/null || trueMakefile variable shared across both files to avoid drift if the cleanup logic ever changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 126 - 132, The TCP 9002 cleanup trap is duplicated across the Makefile targets and the standalone make include, so centralize it into a shared variable such as KILL_9002 and reuse it in start, start-backend, start-standalone, and start-standalone-mock. Update the relevant Makefile targets to reference that symbol instead of embedding the trap string inline, keeping the cleanup behavior identical while avoiding drift.web/src/model/views.ts (1)
124-149: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
packetTranslationre-implementsbaseColumnsinstead of reusing it.Every other preset spreads
...baseColumns; this one hand-lists starttime/srcnamespace/srcname/.../dstport/proto, duplicatingbaseColumnscontent (plus srcaddr/dstaddr). IfbaseColumnschanges later, this preset will silently drift out of sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/model/views.ts` around lines 124 - 149, The packetTranslation preset in views.ts is duplicating the baseColumns list instead of reusing it, which can drift out of sync over time. Update the packetTranslation entry to spread baseColumns like the other presets, then add only the extra packetTranslation-specific columns (such as srcaddr, dstaddr, and the Xlat* fields) in the columns array.web/src/components/__tests-data__/panels.ts (1)
7-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale name:
ShuffledDefaultPanelsis no longer shuffled.The fixture is now a fixed, explicit list (good for determinism), but keeping the "Shuffled" name is misleading now that randomness was intentionally removed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/components/__tests-data__/panels.ts` around lines 7 - 14, The fixture name in panels.ts is misleading because ShuffledDefaultPanels is no longer shuffled and is now a fixed deterministic list. Rename the exported constant to something that reflects its stable ordering, and update any references that use ShuffledDefaultPanels so the test data name matches its actual purpose.web/src/utils/router.ts (1)
200-206: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTighten
setURLViewparameter type toViewPresetId.
viewIdis typed as plainstring, but every caller passes aViewPresetId. Using the narrower type catches typos/invalid preset ids at compile time, consistent with howsetURLMetricTypetypes its param asMetricType.♻️ Suggested fix
-export const setURLView = (viewId: string, replace?: boolean) => { +export const setURLView = (viewId: ViewPresetId, replace?: boolean) => {As per path instructions, "Follow TypeScript type-safety standards... in plugin-mode frontend code."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/utils/router.ts` around lines 200 - 206, `setURLView` is currently typed with a broad string for `viewId`, but all callers use `ViewPresetId`; tighten the parameter type in `setURLView` to `ViewPresetId` so invalid preset ids are caught at compile time. Keep the existing logic and align the typing with `setURLMetricType` by updating the function signature only, using the `ViewPresetId` symbol from `router.ts`.Source: Path instructions
web/src/utils/netflow-capabilities-hook.ts (1)
154-164: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate
getViewPreset(activeView)lookups.Both
selectedPanelsandselectedColumnsindependently callgetViewPreset(activeView). Hoisting into a shared memo avoids the duplicate array scan and keeps both derivations in sync if the preset ever needs additional handling.♻️ Suggested refactor
+ const activePreset = React.useMemo( + () => (activeView !== 'all' ? getViewPreset(activeView) : undefined), + [activeView] + ); + const selectedPanels = React.useMemo(() => { - const preset = activeView !== 'all' ? getViewPreset(activeView) : undefined; - if (preset?.panels) { + if (activePreset?.panels) { return availablePanels.filter( - panel => preset.panels!.includes(panel.id) || (panel.isSelected && !defaultPanelIds.includes(panel.id)) + panel => activePreset.panels!.includes(panel.id) || (panel.isSelected && !defaultPanelIds.includes(panel.id)) ); } return availablePanels.filter(panel => panel.isSelected); - }, [availablePanels, activeView]); + }, [availablePanels, activePreset]);Also applies to: 176-192
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/utils/netflow-capabilities-hook.ts` around lines 154 - 164, Hoist the active view preset lookup into a shared React memo so `selectedPanels` and `selectedColumns` don’t both call `getViewPreset(activeView)` separately. Add a single derived `preset` value near the two `useMemo` blocks, then have both computations read from that shared value so the preset is scanned once and both selections stay aligned in `netflow-capabilities-hook`.web/src/components/__tests__/netflow-traffic.spec.tsx (1)
70-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest coverage regression: call-count-only assertions.
Replacing
toHaveBeenNthCalledWith(...)argument checks with plain call-count assertions loses verification that the correct query payloads (fields/scope) are sent for the default "All Traffic" view. Consider keeping at least one argument-level assertion per call to catch query-construction regressions, not just count regressions.Also applies to: 87-88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/components/__tests__/netflow-traffic.spec.tsx` around lines 70 - 76, The test coverage for the default “All Traffic” path only checks call counts now, which misses regressions in the query payloads. In the netflow-traffic spec, keep at least one argument-level assertion for the mocked fetches in the waitFor block around getMetricsMock and getGenericMetricsMock, verifying the expected fields/scope for the base panels instead of relying only on invocation counts. Apply the same fix to the other affected assertions so the test continues to validate both the number of calls and the constructed query parameters.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/src/components/dropdowns/view-selector.tsx`:
- Line 28: The fallback label in the view-selector component is being translated
twice: activeLabel currently defaults to a pre-translated string and is then
passed again into t(activeLabel) during render. Update activeLabel to hold the
raw i18n key for the default view instead of calling t() there, and keep the
single translation call at render time using t(...) so the label is translated
only once.
- Around line 33-46: The `data-test="view-selector-dropdown"` is currently
attached to `Select`, but the clickable element is `MenuToggle` inside the
`toggle` render prop, so E2E selectors may miss it. Move the test attribute onto
`MenuToggle` in `view-selector.tsx` (or pass it through `toggleProps` if
supported) so the actual toggle button is targeted consistently.
In `@web/src/components/netflow-traffic.tsx`:
- Around line 300-321: Preset selection is permanently persisting the preset’s
topology metric type through applyView and updateTopologyMetricType, so the
user’s original preference is lost when returning to “all”. Update
netflow-traffic.tsx so applyView captures the current topologyMetricType before
applying a preset, and restore that saved value when viewId becomes 'all'
(similar to the panels/columns behavior). Use applyView,
updateTopologyMetricType, and the activeView/reset flow to keep preset-specific
changes temporary rather than writing them permanently to localStorage.
In `@web/src/model/views.ts`:
- Around line 111-122: The networkEvents preset in views.ts is missing the
dedicated NetworkEvents column, so update the preset definition in the
networkEvents view configuration to include the raw 'NetworkEvents' column
alongside the existing baseColumns and ColumnsId entries. Use the existing
preset object with id 'networkEvents' and columns array as the place to add it,
since NetworkEvents is not part of ColumnsId.
---
Nitpick comments:
In `@Makefile`:
- Around line 126-132: The TCP 9002 cleanup trap is duplicated across the
Makefile targets and the standalone make include, so centralize it into a shared
variable such as KILL_9002 and reuse it in start, start-backend,
start-standalone, and start-standalone-mock. Update the relevant Makefile
targets to reference that symbol instead of embedding the trap string inline,
keeping the cleanup behavior identical while avoiding drift.
In `@web/src/components/__tests__/netflow-traffic.spec.tsx`:
- Around line 70-76: The test coverage for the default “All Traffic” path only
checks call counts now, which misses regressions in the query payloads. In the
netflow-traffic spec, keep at least one argument-level assertion for the mocked
fetches in the waitFor block around getMetricsMock and getGenericMetricsMock,
verifying the expected fields/scope for the base panels instead of relying only
on invocation counts. Apply the same fix to the other affected assertions so the
test continues to validate both the number of calls and the constructed query
parameters.
In `@web/src/components/__tests-data__/panels.ts`:
- Around line 7-14: The fixture name in panels.ts is misleading because
ShuffledDefaultPanels is no longer shuffled and is now a fixed deterministic
list. Rename the exported constant to something that reflects its stable
ordering, and update any references that use ShuffledDefaultPanels so the test
data name matches its actual purpose.
In `@web/src/model/views.ts`:
- Around line 124-149: The packetTranslation preset in views.ts is duplicating
the baseColumns list instead of reusing it, which can drift out of sync over
time. Update the packetTranslation entry to spread baseColumns like the other
presets, then add only the extra packetTranslation-specific columns (such as
srcaddr, dstaddr, and the Xlat* fields) in the columns array.
In `@web/src/utils/netflow-capabilities-hook.ts`:
- Around line 154-164: Hoist the active view preset lookup into a shared React
memo so `selectedPanels` and `selectedColumns` don’t both call
`getViewPreset(activeView)` separately. Add a single derived `preset` value near
the two `useMemo` blocks, then have both computations read from that shared
value so the preset is scanned once and both selections stay aligned in
`netflow-capabilities-hook`.
In `@web/src/utils/router.ts`:
- Around line 200-206: `setURLView` is currently typed with a broad string for
`viewId`, but all callers use `ViewPresetId`; tighten the parameter type in
`setURLView` to `ViewPresetId` so invalid preset ids are caught at compile time.
Keep the existing logic and align the typing with `setURLMetricType` by updating
the function signature only, using the `ViewPresetId` symbol from `router.ts`.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 694d62f2-1da4-4b4e-8133-e38bd2b5c1f3
📒 Files selected for processing (19)
.mk/standalone.mkAGENTS.mdMakefileconfig/sample-config.yamlweb/locales/en/plugin__netobserv-plugin.jsonweb/src/components/__tests-data__/panels.tsweb/src/components/__tests__/netflow-traffic.spec.tsxweb/src/components/dropdowns/view-selector.tsxweb/src/components/netflow-traffic.tsxweb/src/model/__tests__/views.spec.tsweb/src/model/netflow-context.tsweb/src/model/views.tsweb/src/utils/config-validation-hook.tsweb/src/utils/local-storage-hook.tsweb/src/utils/netflow-capabilities-hook.tsweb/src/utils/overview-panels.tsweb/src/utils/router.tsweb/src/utils/url-sync-hook.tsweb/src/utils/url.ts
| setOpen(false); | ||
| }; | ||
|
|
||
| const activeLabel = availableViews.find(v => v.id === activeView)?.label ?? t('All Traffic'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Double-translation on the default label fallback.
activeLabel is assigned t('All Traffic') (already translated) when no matching preset is found, then translated again at line 42 via t(activeLabel). Only one t() call should happen. This should use the raw i18n key, not a pre-translated string, so the single call at render time does the translation.
🔧 Proposed fix
- const activeLabel = availableViews.find(v => v.id === activeView)?.label ?? t('All Traffic');
+ const activeLabel = availableViews.find(v => v.id === activeView)?.label ?? 'All Traffic';Also applies to: 42-42
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/dropdowns/view-selector.tsx` at line 28, The fallback
label in the view-selector component is being translated twice: activeLabel
currently defaults to a pre-translated string and is then passed again into
t(activeLabel) during render. Update activeLabel to hold the raw i18n key for
the default view instead of calling t() there, and keep the single translation
call at render time using t(...) so the label is translated only once.
| <Select | ||
| data-test="view-selector-dropdown" | ||
| id="view-selector-dropdown" | ||
| isOpen={isOpen} | ||
| onSelect={onSelect} | ||
| selected={activeView} | ||
| toggle={(toggleRef: React.Ref<MenuToggleElement>) => ( | ||
| <MenuToggle ref={toggleRef} onClick={() => setOpen(!isOpen)} isExpanded={isOpen}> | ||
| <> | ||
| {t('View')}: {t(activeLabel)} | ||
| {isNonDefault && <Badge isRead>{1}</Badge>} | ||
| </> | ||
| </MenuToggle> | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does PatternFly v6 Select component forward unknown/data-* props to the toggle button or to an outer wrapper div?
💡 Result:
In PatternFly v6, the Select component does not directly forward unknown or data-* properties to a wrapper div or a toggle button. Instead, the Select component is a higher-level composite component that manages state and renders sub-components [1][2]. The Select component uses a MenuToggle as its trigger [3][4][5]. If you wish to pass additional properties, such as data-* attributes, to the toggle button, you must use the toggleProps property provided by the Select component [6][7]. Properties passed via toggleProps are forwarded to the underlying MenuToggle component, which in turn passes them to its rendered button element [8][9]. The Select component itself accepts props that configure the menu and its interaction, but general attribute forwarding (like data-*) to the Select's root element is not standard behavior for this component [4][1][2]. If you need to attach data attributes to the Select's outer container, you should check the component's documentation for specific wrapper-related properties or apply them to a custom wrapper element surrounding the Select component.
Citations:
- 1: https://github.com/patternfly/patternfly-react/blob/master/packages/react-core/src/components/Select/Select.tsx
- 2: https://www.patternfly.org/components/menus/select/
- 3: https://www.patternfly.org/components/menus/select
- 4: https://staging-v6.patternfly.org/components/menus/select
- 5: https://raw.githubusercontent.com/patternfly/patternfly-react/refs/tags/v6.4.0/packages/react-core/src/components/Select/examples/Select.md
- 6: https://staging-v6.patternfly.org/components/menus/select/react-templates/
- 7: https://www.patternfly.org/components/menus/select/react-templates/
- 8: https://www.patternfly.org/components/menus/menu-toggle/
- 9: patternfly/patternfly-react@20ae2dc
Put data-test on the actual toggle
data-test="view-selector-dropdown" on <Select> is not guaranteed to reach the clickable <MenuToggle>. Move it to <MenuToggle> (or toggleProps) so E2E selectors target the button.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/dropdowns/view-selector.tsx` around lines 33 - 46, The
`data-test="view-selector-dropdown"` is currently attached to `Select`, but the
clickable element is `MenuToggle` inside the `toggle` render prop, so E2E
selectors may miss it. Move the test attribute onto `MenuToggle` in
`view-selector.tsx` (or pass it through `toggleProps` if supported) so the
actual toggle button is targeted consistently.
Source: Path instructions
| const resetDefaultFilters = React.useCallback(() => { | ||
| setActiveView('all'); | ||
| updateTableFilters({ match: filters.match, list: caps.defaultFilters }); | ||
| }, [filters.match, caps.defaultFilters, updateTableFilters]); | ||
| }, [filters.match, caps.defaultFilters, updateTableFilters, setActiveView]); | ||
|
|
||
| const applyView = React.useCallback( | ||
| (viewId: ViewPresetId) => { | ||
| setActiveView(viewId); | ||
| if (viewId === 'all') { | ||
| // Don't reset panels/columns — restore user's custom localStorage selection | ||
| return; | ||
| } | ||
| const preset = getViewPreset(viewId); | ||
| if (!preset) { | ||
| return; | ||
| } | ||
| if (preset.topologyMetricType) { | ||
| updateTopologyMetricType(preset.topologyMetricType); | ||
| } | ||
| }, | ||
| [setActiveView, updateTopologyMetricType] | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preset selection permanently overwrites the user's saved topology metric type.
applyView calls updateTopologyMetricType(preset.topologyMetricType), which persists to localStorage. Unlike panels/columns (explicitly preserved per the comment on line 309), there's no capture/restore of the user's prior topologyMetricType when switching back to 'all'. Once a preset with a fixed topologyMetricType is applied, the user's original preference is lost — switching back to "All Traffic" won't recover it.
🔧 Proposed fix: capture/restore metric type around preset application
+ const previousTopologyMetricType = React.useRef<MetricType | undefined>(undefined);
+
const applyView = React.useCallback(
(viewId: ViewPresetId) => {
setActiveView(viewId);
if (viewId === 'all') {
- // Don't reset panels/columns — restore user's custom localStorage selection
+ // Don't reset panels/columns — restore user's custom localStorage selection
+ if (previousTopologyMetricType.current !== undefined) {
+ updateTopologyMetricType(previousTopologyMetricType.current);
+ previousTopologyMetricType.current = undefined;
+ }
return;
}
const preset = getViewPreset(viewId);
if (!preset) {
return;
}
if (preset.topologyMetricType) {
+ if (previousTopologyMetricType.current === undefined) {
+ previousTopologyMetricType.current = topologyMetricType;
+ }
updateTopologyMetricType(preset.topologyMetricType);
}
},
- [setActiveView, updateTopologyMetricType]
+ [setActiveView, updateTopologyMetricType, topologyMetricType]
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const resetDefaultFilters = React.useCallback(() => { | |
| setActiveView('all'); | |
| updateTableFilters({ match: filters.match, list: caps.defaultFilters }); | |
| }, [filters.match, caps.defaultFilters, updateTableFilters]); | |
| }, [filters.match, caps.defaultFilters, updateTableFilters, setActiveView]); | |
| const applyView = React.useCallback( | |
| (viewId: ViewPresetId) => { | |
| setActiveView(viewId); | |
| if (viewId === 'all') { | |
| // Don't reset panels/columns — restore user's custom localStorage selection | |
| return; | |
| } | |
| const preset = getViewPreset(viewId); | |
| if (!preset) { | |
| return; | |
| } | |
| if (preset.topologyMetricType) { | |
| updateTopologyMetricType(preset.topologyMetricType); | |
| } | |
| }, | |
| [setActiveView, updateTopologyMetricType] | |
| ); | |
| const resetDefaultFilters = React.useCallback(() => { | |
| setActiveView('all'); | |
| updateTableFilters({ match: filters.match, list: caps.defaultFilters }); | |
| }, [filters.match, caps.defaultFilters, updateTableFilters, setActiveView]); | |
| const previousTopologyMetricType = React.useRef<MetricType | undefined>(undefined); | |
| const applyView = React.useCallback( | |
| (viewId: ViewPresetId) => { | |
| setActiveView(viewId); | |
| if (viewId === 'all') { | |
| // Don't reset panels/columns — restore user's custom localStorage selection | |
| if (previousTopologyMetricType.current !== undefined) { | |
| updateTopologyMetricType(previousTopologyMetricType.current); | |
| previousTopologyMetricType.current = undefined; | |
| } | |
| return; | |
| } | |
| const preset = getViewPreset(viewId); | |
| if (!preset) { | |
| return; | |
| } | |
| if (preset.topologyMetricType) { | |
| if (previousTopologyMetricType.current === undefined) { | |
| previousTopologyMetricType.current = topologyMetricType; | |
| } | |
| updateTopologyMetricType(preset.topologyMetricType); | |
| } | |
| }, | |
| [setActiveView, updateTopologyMetricType, topologyMetricType] | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/netflow-traffic.tsx` around lines 300 - 321, Preset
selection is permanently persisting the preset’s topology metric type through
applyView and updateTopologyMetricType, so the user’s original preference is
lost when returning to “all”. Update netflow-traffic.tsx so applyView captures
the current topologyMetricType before applying a preset, and restore that saved
value when viewId becomes 'all' (similar to the panels/columns behavior). Use
applyView, updateTopologyMetricType, and the activeView/reset flow to keep
preset-specific changes temporary rather than writing them permanently to
localStorage.
| { | ||
| id: 'networkEvents', | ||
| label: 'Network Events', | ||
| requiredFeature: 'networkEvents', | ||
| panels: [ | ||
| 'top_avg_dropped_packet_rates', | ||
| 'dropped_packet_rates', | ||
| 'state_dropped_packet_rates', | ||
| 'cause_dropped_packet_rates' | ||
| ], | ||
| columns: [...baseColumns, ColumnsId.bytes, ColumnsId.packets, ColumnsId.dropstate, ColumnsId.dropcause] | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the exact ColumnsId enum member for the NetworkEvents column
rg -n -C2 'networkevents|NetworkEvents' web/src/utils/columns.tsRepository: netobserv/netobserv-web-console
Length of output: 169
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files 'web/src/model/views.ts' 'web/src/utils/*' 'config/*' | sed 's#^`#-` #'
printf '\n== outline views.ts ==\n'
ast-grep outline web/src/model/views.ts --view expanded || true
printf '\n== relevant slices from views.ts ==\n'
sed -n '1,240p' web/src/model/views.ts | cat -n
printf '\n== search for NetworkEvents / networkEvents across repo ==\n'
rg -n -C 2 'NetworkEvents|networkEvents' web/src config web -g '!**/node_modules/**' || true
printf '\n== outline columns utils if present ==\n'
fd -a 'columns.ts' web/src || true
fd -a 'columns.*' web/src || trueRepository: netobserv/netobserv-web-console
Length of output: 22573
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== columns.ts outline ==\n'
ast-grep outline web/src/utils/columns.ts --view expanded || true
printf '\n== columns.ts relevant contents ==\n'
sed -n '1,260p' web/src/utils/columns.ts | cat -n
printf '\n== columns tests references ==\n'
sed -n '1,220p' web/src/utils/__tests__/columns.spec.ts | cat -n
printf '\n== search for NetworkEvents column ID in UI code ==\n'
rg -n -C 2 'ColumnsId\.|NetworkEvents|network events' web/src web/cypress config -g '!**/node_modules/**' || trueRepository: netobserv/netobserv-web-console
Length of output: 45104
Add NetworkEvents to the networkEvents preset (web/src/model/views.ts:111-122).
config/sample-config.yaml defines a dedicated NetworkEvents column, but this preset omits it, so the network-events view won't show the event payload by default. Use the raw string 'NetworkEvents' here; it isn't in ColumnsId.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/model/views.ts` around lines 111 - 122, The networkEvents preset in
views.ts is missing the dedicated NetworkEvents column, so update the preset
definition in the networkEvents view configuration to include the raw
'NetworkEvents' column alongside the existing baseColumns and ColumnsId entries.
Use the existing preset object with id 'networkEvents' and columns array as the
place to add it, since NetworkEvents is not part of ColumnsId.
|
/ok-to-test |
openshift-ci
Bot
added
the
ok-to-test
|
New images: quay.io/netobserv/network-observability-console-plugin:99901ccb
quay.io/netobserv/network-observability-standalone-frontend:99901ccbThey will expire in two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=99901ccb make set-plugin-image |
memodi
removed
the
ok-to-test
|
/ok-to-test |
openshift-ci
Bot
added
the
ok-to-test
|
New images: quay.io/netobserv/network-observability-console-plugin:99901ccb
quay.io/netobserv/network-observability-standalone-frontend:99901ccbThey will expire in two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=99901ccb make set-plugin-image |
|
@memodi: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
github-actions
Bot
removed
the
ok-to-test
1 participant
Description
Feature views helps users with preset columns/panels/metrics for each feature that's enabled in Flowcollector dynamically.
User journey are listed here
Co-Authored-By: Claude Sonnet.
Dependencies
netobserv/netobserv-operator#2836
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Documentation