Skip to content

CNV-74280: migrate vmi, network and templates tables#3561

Open
batyana wants to merge 1 commit intokubevirt-ui:mainfrom
batyana:CNV-74280-pr7-simple-tables
Open

CNV-74280: migrate vmi, network and templates tables#3561
batyana wants to merge 1 commit intokubevirt-ui:mainfrom
batyana:CNV-74280-pr7-simple-tables

Conversation

@batyana
Copy link
Member

@batyana batyana commented Mar 4, 2026

📝 Description

Summary

  • Migrates 5 simple tables from deprecated VirtualizedTable to KubevirtTable wrapper
  • Part of CNV-74280 table migration effort
  • Tables: VMI DisksTable, VMNetworkList, VMNetworkOtherTypesList, NodesTable, TemplatesCatalogItems

🎥 Demo

Screen.Recording.2026-03-10.at.13.44.26.mov

Summary by CodeRabbit

  • Documentation

    • Added translations (EN, ES, FR, JA, KO, ZH) for table labels and empty-state messages.
  • New Features

    • New table labels for Disks, Nodes, VM Networks, Other VM Networks, and Templates catalog.
    • Templates catalog now receives unfiltered template data.
    • Improved search input accessibility (aria-label).
  • Bug Fixes

    • Clearer "no data" / "no results" messaging and more reliable empty-state behavior across templates, nodes, disks, and VM network views.

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Mar 4, 2026

@batyana: This pull request references CNV-74280 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

📝 Description

Summary

  • Migrates 5 simple tables from deprecated VirtualizedTable to KubevirtTable wrapper
  • Part of CNV-74280 table migration effort
  • Tables: VMI DisksTable, VMNetworkList, VMNetworkOtherTypesList, NodesTable, TemplatesCatalogItems

🎥 Demo

Please add a video or an image of the behavior/changes

Instructions 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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 4, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 4, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: batyana
Once this PR has been reviewed and has the lgtm label, please assign galkremer1 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai
Copy link

coderabbitai bot commented Mar 4, 2026

📝 Walkthrough

Walkthrough

Replaces per-row components and column hooks with centralized ColumnConfig modules and KubevirtTable, adds new table/empty-state localization keys across locales, and introduces table-definition modules and cell renderers for templates, nodes, disks, and VM networks.

Changes

Cohort / File(s) Summary
Localization Entries
locales/en/plugin__kubevirt-plugin.json, locales/es/plugin__kubevirt-plugin.json, locales/fr/plugin__kubevirt-plugin.json, locales/ja/plugin__kubevirt-plugin.json, locales/ko/plugin__kubevirt-plugin.json, locales/zh/plugin__kubevirt-plugin.json
Added ~10 new translation keys: table labels, empty-state messages, node selection prompt, and used/total cores strings across locales.
Templates catalog
src/views/catalog/templatescatalog/components/TemplatesCatalogItems.tsx, src/views/catalog/templatescatalog/components/TemplatesCatalogRow.tsx, src/views/catalog/templatescatalog/hooks/useTemplatesCatalogColumns.ts, src/views/catalog/templatescatalog/templatesCatalogTableDefinition.tsx, src/views/catalog/templatescatalog/templatesCatalogCells.tsx, src/views/catalog/templatescatalog/types.ts, src/views/catalog/templatescatalog/TemplatesCatalog.tsx
Removed legacy row component and hook; added centralized table definition, cell components (NameCell, SourceCell, CPUMemoryCell), callbacks type, getTemplatesCatalogColumns/getTemplateCatalogRowId, and passed unfilteredTemplates prop; switched to KubevirtTable.
Nodes table (VM compute migration)
src/views/virtualmachines/.../NodesTable/NodesTable.tsx, src/views/virtualmachines/.../NodesTable/components/NodeRow.tsx, .../NodeRow.scss, .../utils/hooks/useNodesColumns.ts, .../nodesTableDefinition.tsx
Replaced VirtualizedTable with KubevirtTable, removed NodeRow and useNodesColumns, added nodesTableDefinition exporting getNodesTableColumns, getNodeRowId and NodesTableCallbacks; updated search/filter handling and selection callbacks.
Disks table (VMI details)
src/views/virtualmachinesinstance/details/tabs/disks/table/disks/DisksTable.tsx, .../DisksTableRow.tsx, .../hooks/useDisksTableColumns.ts, .../disksTableDefinition.tsx
Switched to KubevirtTable, removed DisksTableRow and useDisksTableColumns hook, added disksTableDefinition with getVMIDisksTableColumns and getVMIDiskRowId and cell renderers for Source and Size.
VM Networks lists
src/views/vmnetworks/list/VMNetworkList.tsx, src/views/vmnetworks/list/VMNetworkOtherTypesList.tsx, src/views/vmnetworks/list/components/VMNetworkRow.tsx, src/views/vmnetworks/list/components/VMNetworkOtherRow.tsx, src/views/vmnetworks/list/hooks/useVMNetworkColumns.tsx, src/views/vmnetworks/list/vmNetworkListDefinition.tsx, src/views/vmnetworks/list/vmNetworkOtherTypesListDefinition.tsx
Removed per-row components and hook; added vmNetworkListDefinition and vmNetworkOtherTypesListDefinition exporting getVMNetworkListColumns/getVMNetworkRowId and getVMNetworkOtherTypesColumns/getVMNetworkOtherRowId; migrated lists to KubevirtTable and updated empty-state logic.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly references the main change: migration of five tables (vmi, network, templates) from VirtualizedTable to KubevirtTable, aligning with the CNV-74280 effort.
Description check ✅ Passed The description includes a summary of the migration effort with specific tables listed and a reference to the Jira issue, but lacks a visual demo as prompted by the template.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can generate a title for your PR based on the changes.

Add @coderabbitai placeholder anywhere in the title of your PR and CodeRabbit will replace it with a title based on the changes in the PR. You can change the placeholder by changing the reviews.auto_title_placeholder setting.

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Mar 4, 2026

@batyana: This pull request references CNV-74280 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

📝 Description

Summary

  • Migrates 5 simple tables from deprecated VirtualizedTable to KubevirtTable wrapper
  • Part of CNV-74280 table migration effort
  • Tables: VMI DisksTable, VMNetworkList, VMNetworkOtherTypesList, NodesTable, TemplatesCatalogItems

🎥 Demo

Please add a video or an image of the behavior/changes

Instructions 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 openshift-eng/jira-lifecycle-plugin repository.

@batyana batyana force-pushed the CNV-74280-pr7-simple-tables branch 3 times, most recently from 59fd5b5 to 2a829b4 Compare March 10, 2026 11:51
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Mar 10, 2026

@batyana: This pull request references CNV-74280 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

📝 Description

Summary

  • Migrates 5 simple tables from deprecated VirtualizedTable to KubevirtTable wrapper
  • Part of CNV-74280 table migration effort
  • Tables: VMI DisksTable, VMNetworkList, VMNetworkOtherTypesList, NodesTable, TemplatesCatalogItems

🎥 Demo

Screen.Recording.2026-03-10.at.13.44.26.mov

Instructions 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 openshift-eng/jira-lifecycle-plugin repository.

@batyana batyana marked this pull request as ready for review March 10, 2026 11:53
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Mar 10, 2026

@batyana: This pull request references CNV-74280 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

📝 Description

Summary

  • Migrates 5 simple tables from deprecated VirtualizedTable to KubevirtTable wrapper
  • Part of CNV-74280 table migration effort
  • Tables: VMI DisksTable, VMNetworkList, VMNetworkOtherTypesList, NodesTable, TemplatesCatalogItems

🎥 Demo

Screen.Recording.2026-03-10.at.13.44.26.mov

Summary by CodeRabbit

  • Documentation

  • Added localization support across English, Spanish, French, Japanese, Korean, and Chinese for table labels and empty state messages.

  • New Features

  • Enhanced search functionality with improved accessibility features.

  • Added "no results" and "no data found" messaging for better user feedback when lists are empty or filtered.

  • Bug Fixes

  • Improved empty state handling and data loading indicators across templates, nodes, disks, and VM network views.

Instructions 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 openshift-eng/jira-lifecycle-plugin repository.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (3)
src/views/virtualmachinesinstance/details/tabs/disks/table/disks/disksTableDefinition.tsx (1)

25-25: Use data-test consistently for the new cell selectors.

These new spans introduce data-test-id, while the repo standard for selectors in src/**/*.{ts,tsx} is data-test. Keeping the attribute consistent avoids splitting test conventions across the migrated tables.

Proposed fix
-  return <span data-test-id={`disk-source-${disk.name}`}>{disk.source ?? NO_DATA_DASH}</span>;
+  return <span data-test={`disk-source-${disk.name}`}>{disk.source ?? NO_DATA_DASH}</span>;

-  return <span data-test-id={`disk-size-${disk.name}`}>{size ?? disk.size ?? NO_DATA_DASH}</span>;
+  return <span data-test={`disk-size-${disk.name}`}>{size ?? disk.size ?? NO_DATA_DASH}</span>;

-      <span data-test-id={`disk-name-${row.name}`}>{row.name ?? NO_DATA_DASH}</span>
+      <span data-test={`disk-name-${row.name}`}>{row.name ?? NO_DATA_DASH}</span>

-      <span data-test-id={`disk-drive-${row.name}`}>{row.drive ?? NO_DATA_DASH}</span>
+      <span data-test={`disk-drive-${row.name}`}>{row.drive ?? NO_DATA_DASH}</span>

-      <span data-test-id={`disk-interface-${row.name}`}>{row.interface ?? NO_DATA_DASH}</span>
+      <span data-test={`disk-interface-${row.name}`}>{row.interface ?? NO_DATA_DASH}</span>

-      <span data-test-id={`disk-storageclass-${row.name}`}>{row.storageClass ?? NO_DATA_DASH}</span>
+      <span data-test={`disk-storageclass-${row.name}`}>{row.storageClass ?? NO_DATA_DASH}</span>

As per coding guidelines, src/**/*.{ts,tsx}: Use data-test attributes for selectors in tests instead of implementation details.

Also applies to: 30-30, 39-39, 62-62, 71-71, 80-80

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/views/virtualmachinesinstance/details/tabs/disks/table/disks/disksTableDefinition.tsx`
at line 25, In disksTableDefinition (the cell renderers returning spans for disk
fields), replace the nonstandard data-test-id attributes with the repo-standard
data-test attributes; e.g., change data-test-id={`disk-source-${disk.name}`} to
data-test={`disk-source-${disk.name}`} and do the same for the other cell spans
(disk-size-..., disk-type-..., disk-bootable-..., disk-interface-...) so all
selectors use data-test consistently.
src/views/catalog/templatescatalog/templatesCatalogTableDefinition.tsx (1)

1-156: Split this new module before it grows further.

It already exceeds the repo's 150-line ceiling. Extracting the cell renderers or helper types into sibling files would keep the table definition focused on column config.

As per coding guidelines, src/**/*.{ts,tsx} files have a maximum size of 150 lines and should be split once they exceed it.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/views/catalog/templatescatalog/templatesCatalogTableDefinition.tsx`
around lines 1 - 156, This file exceeds the 150-line limit; split out the cell
components and helper types to keep the table focused: create sibling modules
(e.g., NameCell.tsx, SourceCell.tsx, CPUMemoryCell.tsx and/or
templatesCatalogTypes.ts) and move the corresponding symbols (NameCell,
SourceCell, CPUMemoryCell, renderNameCell, renderSourceCell,
TemplatesCatalogCallbacks type) into them, export what’s needed, and update this
file to import those symbols and only export getTemplatesCatalogColumns and
getTemplateCatalogRowId; ensure all referenced helpers (getTemplateOSIcon,
getTemplateBootSourceType, getTemplateFlavorData, etc.) are still imported where
used.
src/views/vmnetworks/list/vmNetworkOtherTypesListDefinition.tsx (1)

17-42: Consider adding explicit return types for consistency.

The helper functions renderNameCell and renderNamespaceCell lack explicit return types. While the return type is inferable, the coding guidelines recommend explicit return types for functions.

🔧 Suggested improvement
-const renderNameCell = (row: OtherVMNetworkWithType): ReactNode => {
+const renderNameCell = (row: OtherVMNetworkWithType): ReactNode => {
   const name = getName(row);
   const namespace = getNamespace(row);
   // ... rest unchanged
 };

-const renderNamespaceCell = (row: OtherVMNetworkWithType): ReactNode => {
+const renderNamespaceCell = (row: OtherVMNetworkWithType): ReactNode => {

Note: The current code already has : ReactNode - disregard if this is just a display issue. The explicit return types are already present on lines 17 and 32.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/views/vmnetworks/list/vmNetworkOtherTypesListDefinition.tsx` around lines
17 - 42, The reviewer noted missing explicit return types for renderNameCell and
renderNamespaceCell; ensure both functions explicitly declare their return type
(e.g., append ": ReactNode" to the signatures of renderNameCell and
renderNamespaceCell), confirm the ReactNode type is imported from React and that
the parameter types (OtherVMNetworkWithType) remain correct, or if the
annotations already exist (display glitch) remove the inconsistency by making
both signatures identical and re-running the typecheck.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/views/catalog/templatescatalog/components/TemplatesCatalogItems.tsx`:
- Around line 53-67: The KubevirtTable is receiving the already-filtered
templates array for both data and unfilteredData inside TemplatesCatalogItems,
so the table cannot tell an empty catalog from a filter-empty result; change the
unfilteredData prop to pass the original pre-filter catalog array (the
upstream/original catalog variable you receive in TemplatesCatalogItems) instead
of templates, or alternatively move filtering into this component and keep
unfilteredData as the original catalog; update the KubevirtTable call (prop
unfilteredData) accordingly and ensure any prop name for the original catalog is
threaded through to TemplatesCatalogItems so filters.isList logic can compare
against the true unfiltered set.

In `@src/views/catalog/templatescatalog/templatesCatalogTableDefinition.tsx`:
- Around line 44-61: The Name column displays getTemplateName(template) (which
prefers the display-name annotation) but sorting uses getValue/metadata.name,
causing mismatch; update the sort key to use the same visible value by changing
the comparator used in getValue (and the grid sort in TemplatesCatalogItems.tsx)
to derive the sort value from getTemplateName(template) or the display-name
annotation (ANNOTATIONS.displayName) when present, ensuring NameCell,
getTemplateName, and the grid sort logic all use the same display string for
sorting.
- Around line 69-79: The lookup into availableDatasources using
`${bootSource?.source?.sourceRef?.namespace}-${bootSource?.source?.sourceRef?.name}`
can collide (e.g., foo-bar/baz vs foo/bar-baz); instead ensure the map uses the
unambiguous "namespace/name" key and change the lookup in
templatesCatalogTableDefinition.tsx (where availableDatasources, bootSource, and
dataSource are used and passed to
TemplateRowAvailableSource/getVMBootSourceLabel) to use `${namespace}/${name}`;
if availableDatasources is built elsewhere, update its construction to use
`${ns}/${name}` consistently so the getTemplateBootSourceType -> dataSource
resolution is one-to-one.

In
`@src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/nodesTableDefinition.tsx`:
- Around line 39-49: In CPUCell, replace the hardcoded "cores" with a translated
string from useKubevirtTranslation (use t('cores') or appropriate key) and guard
against NaN from parseFloat(usedCPU) by checking Number.isFinite(parsed) (or
isFinite) before calling toFixed; if parsed usedCPU is invalid show the
translated N/A (t('N/A')) or another suitable translated fallback, then render
the final string as "{formattedUsed} {t('cores')} / {totalCPU} {t('cores')}"
while keeping the existing data-test-id and using the CPUCell, usedCPU, totalCPU
and t symbols to locate the change.

In
`@src/views/virtualmachinesinstance/details/tabs/disks/table/disks/disksTableDefinition.tsx`:
- Around line 18-22: The Source column currently passes disk.name to
ResourceLink causing PVC-backed rows to link to the disk instead of the claim;
update the ResourceLink in disksTableDefinition (the ResourceLink invocation
that uses modelToGroupVersionKind(PersistentVolumeClaimModel)) to pass
disk.source (or disk.source || disk.name as a safe fallback) as the name prop
for PVC rows so the link navigates to the PVC name; keep the existing namespace
and groupVersionKind usage unchanged.

---

Nitpick comments:
In `@src/views/catalog/templatescatalog/templatesCatalogTableDefinition.tsx`:
- Around line 1-156: This file exceeds the 150-line limit; split out the cell
components and helper types to keep the table focused: create sibling modules
(e.g., NameCell.tsx, SourceCell.tsx, CPUMemoryCell.tsx and/or
templatesCatalogTypes.ts) and move the corresponding symbols (NameCell,
SourceCell, CPUMemoryCell, renderNameCell, renderSourceCell,
TemplatesCatalogCallbacks type) into them, export what’s needed, and update this
file to import those symbols and only export getTemplatesCatalogColumns and
getTemplateCatalogRowId; ensure all referenced helpers (getTemplateOSIcon,
getTemplateBootSourceType, getTemplateFlavorData, etc.) are still imported where
used.

In
`@src/views/virtualmachinesinstance/details/tabs/disks/table/disks/disksTableDefinition.tsx`:
- Line 25: In disksTableDefinition (the cell renderers returning spans for disk
fields), replace the nonstandard data-test-id attributes with the repo-standard
data-test attributes; e.g., change data-test-id={`disk-source-${disk.name}`} to
data-test={`disk-source-${disk.name}`} and do the same for the other cell spans
(disk-size-..., disk-type-..., disk-bootable-..., disk-interface-...) so all
selectors use data-test consistently.

In `@src/views/vmnetworks/list/vmNetworkOtherTypesListDefinition.tsx`:
- Around line 17-42: The reviewer noted missing explicit return types for
renderNameCell and renderNamespaceCell; ensure both functions explicitly declare
their return type (e.g., append ": ReactNode" to the signatures of
renderNameCell and renderNamespaceCell), confirm the ReactNode type is imported
from React and that the parameter types (OtherVMNetworkWithType) remain correct,
or if the annotations already exist (display glitch) remove the inconsistency by
making both signatures identical and re-running the typecheck.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0854fd54-91ae-4a74-98d2-b8e2b66c8357

📥 Commits

Reviewing files that changed from the base of the PR and between 4b644a0 and 2a829b4.

📒 Files selected for processing (26)
  • locales/en/plugin__kubevirt-plugin.json
  • locales/es/plugin__kubevirt-plugin.json
  • locales/fr/plugin__kubevirt-plugin.json
  • locales/ja/plugin__kubevirt-plugin.json
  • locales/ko/plugin__kubevirt-plugin.json
  • locales/zh/plugin__kubevirt-plugin.json
  • src/views/catalog/templatescatalog/components/TemplatesCatalogItems.tsx
  • src/views/catalog/templatescatalog/components/TemplatesCatalogRow.tsx
  • src/views/catalog/templatescatalog/hooks/useTemplatesCatalogColumns.ts
  • src/views/catalog/templatescatalog/templatesCatalogTableDefinition.tsx
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/NodesTable.tsx
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/components/NodeRow.scss
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/components/NodeRow.tsx
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/nodesTableDefinition.tsx
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/utils/hooks/useNodesColumns.ts
  • src/views/virtualmachinesinstance/details/tabs/disks/hooks/useDisksTableColumns.ts
  • src/views/virtualmachinesinstance/details/tabs/disks/table/disks/DisksTable.tsx
  • src/views/virtualmachinesinstance/details/tabs/disks/table/disks/DisksTableRow.tsx
  • src/views/virtualmachinesinstance/details/tabs/disks/table/disks/disksTableDefinition.tsx
  • src/views/vmnetworks/list/VMNetworkList.tsx
  • src/views/vmnetworks/list/VMNetworkOtherTypesList.tsx
  • src/views/vmnetworks/list/components/VMNetworkOtherRow.tsx
  • src/views/vmnetworks/list/components/VMNetworkRow.tsx
  • src/views/vmnetworks/list/hooks/useVMNetworkColumns.tsx
  • src/views/vmnetworks/list/vmNetworkListDefinition.tsx
  • src/views/vmnetworks/list/vmNetworkOtherTypesListDefinition.tsx
💤 Files with no reviewable changes (10)
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/components/NodeRow.scss
  • src/views/catalog/templatescatalog/hooks/useTemplatesCatalogColumns.ts
  • src/views/vmnetworks/list/components/VMNetworkOtherRow.tsx
  • src/views/vmnetworks/list/components/VMNetworkRow.tsx
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/utils/hooks/useNodesColumns.ts
  • src/views/virtualmachinesinstance/details/tabs/disks/table/disks/DisksTableRow.tsx
  • src/views/catalog/templatescatalog/components/TemplatesCatalogRow.tsx
  • src/views/virtualmachinesinstance/details/tabs/disks/hooks/useDisksTableColumns.ts
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/components/NodeRow.tsx
  • src/views/vmnetworks/list/hooks/useVMNetworkColumns.tsx

Comment on lines +69 to +79
const { availableDatasources, availableTemplatesUID } = callbacks;
const bootSource = getTemplateBootSourceType(template);
const dataSource =
availableDatasources[
`${bootSource?.source?.sourceRef?.namespace}-${bootSource?.source?.sourceRef?.name}`
];

return (
<TemplateRowAvailableSource
isBootSourceAvailable={availableTemplatesUID.has(template?.metadata?.uid)}
source={getVMBootSourceLabel(bootSource?.type, dataSource)}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Use a collision-free key for availableDatasources.

"${namespace}-${name}" is ambiguous for valid Kubernetes identifiers. foo-bar/baz and foo/bar-baz both flatten to the same string, so this lookup can resolve the wrong DataSource for a template.

Suggested direction
   const { availableDatasources, availableTemplatesUID } = callbacks;
   const bootSource = getTemplateBootSourceType(template);
-  const dataSource =
-    availableDatasources[
-      `${bootSource?.source?.sourceRef?.namespace}-${bootSource?.source?.sourceRef?.name}`
-    ];
+  const sourceRef = bootSource?.source?.sourceRef;
+  const dataSourceKey = sourceRef ? `${sourceRef.namespace}/${sourceRef.name}` : undefined;
+  const dataSource = dataSourceKey ? availableDatasources[dataSourceKey] : undefined;

Build availableDatasources with the same namespace/name key so the mapping stays one-to-one.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/views/catalog/templatescatalog/templatesCatalogTableDefinition.tsx`
around lines 69 - 79, The lookup into availableDatasources using
`${bootSource?.source?.sourceRef?.namespace}-${bootSource?.source?.sourceRef?.name}`
can collide (e.g., foo-bar/baz vs foo/bar-baz); instead ensure the map uses the
unambiguous "namespace/name" key and change the lookup in
templatesCatalogTableDefinition.tsx (where availableDatasources, bootSource, and
dataSource are used and passed to
TemplateRowAvailableSource/getVMBootSourceLabel) to use `${namespace}/${name}`;
if availableDatasources is built elsewhere, update its construction to use
`${ns}/${name}` consistently so the getTemplateBootSourceType -> dataSource
resolution is one-to-one.

@batyana batyana force-pushed the CNV-74280-pr7-simple-tables branch from 2a829b4 to bc3a818 Compare March 10, 2026 19:01
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Mar 10, 2026

@batyana: This pull request references CNV-74280 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

📝 Description

Summary

  • Migrates 5 simple tables from deprecated VirtualizedTable to KubevirtTable wrapper
  • Part of CNV-74280 table migration effort
  • Tables: VMI DisksTable, VMNetworkList, VMNetworkOtherTypesList, NodesTable, TemplatesCatalogItems

🎥 Demo

Screen.Recording.2026-03-10.at.13.44.26.mov

Summary by CodeRabbit

  • Documentation

  • Added English, Spanish, French, Japanese, Korean, and Chinese translations for table labels and empty-state messages.

  • New Features

  • New/updated table labels for Disks, Nodes, VM Networks, Other VM Networks, and Templates catalog.

  • Templates catalog now receives full/unfiltered template data.

  • Search input accessibility improved.

  • Bug Fixes

  • Clearer "no data" and "no results" messages and improved empty-state/loading behavior across templates, nodes, disks, and VM network views.

Instructions 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 openshift-eng/jira-lifecycle-plugin repository.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@locales/en/plugin__kubevirt-plugin.json`:
- Line 86: The localization string currently hardcodes the plural "cores" in the
key/value "{{used}} cores / {{total}} cores", which yields incorrect output like
"1 cores"; update the resource so it avoids pluralizing the unit directly—either
change the string to a neutral form such as "{{used}} / {{total}} cores" or
accept fully formatted labels from the caller (e.g., use keys like
"{{usedLabel}} / {{totalLabel}}") and update callers that reference "{{used}}
cores / {{total}} cores" to pass properly pluralized labels (or the neutral
form) instead; ensure you update the JSON key/value and any call-sites that
interpolate used/total accordingly.

In `@src/views/catalog/templatescatalog/types.ts`:
- Around line 5-6: The current types make the lookup collections appear dense
and mutable; change availableDatasources to be sparse and readonly and make
availableTemplatesUID readonly: replace availableDatasources: Record<string,
V1beta1DataSource> with availableDatasources: Readonly<Record<string,
V1beta1DataSource | undefined>> and replace availableTemplatesUID: Set<string>
with availableTemplatesUID: ReadonlySet<string> so callers see possible
undefined entries and cannot mutate the collections.

In
`@src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/nodesTableDefinition.tsx`:
- Around line 78-79: The memory display string is not translated: update the
usedMemoryString construction in nodesTableDefinition.tsx to use the
useKubevirtTranslation() t() call (same pattern as CPUCell) and pass values as
interpolation params (e.g. t('{{usedMemoryValue}} {{usedMemoryUnits}} /
{{totalMemoryValue}} {{totalMemoryUnits}}', { usedMemoryValue, usedMemoryUnits,
totalMemoryValue, totalMemoryUnits })). Replace the current template literal and
ensure the returned span uses the translated string (data-test prop can remain
unchanged).

In `@src/views/vmnetworks/list/VMNetworkOtherTypesList.tsx`:
- Line 48: The current getVMNetworkOtherRowId fallback uses only the
name-namespace tuple (see vmNetworkOtherTypesListDefinition.tsx where fallback
is built with `${name}-${namespace}`), which can collide across different
network kinds; update the fallback in getVMNetworkOtherRowId to include the
resource kind or row.type (e.g., `${row.type}-${name}-${namespace}`) so the
computed row id is unique across mixed resource lists and still prefer
metadata.uid when available. Ensure you update any related helper/consumer names
(getVMNetworkOtherRowId) to use the new fallback format consistently.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4f1e3d54-a201-42b4-8642-420d04600b54

📥 Commits

Reviewing files that changed from the base of the PR and between 2a829b4 and bc3a818.

📒 Files selected for processing (29)
  • locales/en/plugin__kubevirt-plugin.json
  • locales/es/plugin__kubevirt-plugin.json
  • locales/fr/plugin__kubevirt-plugin.json
  • locales/ja/plugin__kubevirt-plugin.json
  • locales/ko/plugin__kubevirt-plugin.json
  • locales/zh/plugin__kubevirt-plugin.json
  • src/views/catalog/templatescatalog/TemplatesCatalog.tsx
  • src/views/catalog/templatescatalog/components/TemplatesCatalogItems.tsx
  • src/views/catalog/templatescatalog/components/TemplatesCatalogRow.tsx
  • src/views/catalog/templatescatalog/hooks/useTemplatesCatalogColumns.ts
  • src/views/catalog/templatescatalog/templatesCatalogCells.tsx
  • src/views/catalog/templatescatalog/templatesCatalogTableDefinition.tsx
  • src/views/catalog/templatescatalog/types.ts
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/NodesTable.tsx
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/components/NodeRow.scss
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/components/NodeRow.tsx
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/nodesTableDefinition.tsx
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/utils/hooks/useNodesColumns.ts
  • src/views/virtualmachinesinstance/details/tabs/disks/hooks/useDisksTableColumns.ts
  • src/views/virtualmachinesinstance/details/tabs/disks/table/disks/DisksTable.tsx
  • src/views/virtualmachinesinstance/details/tabs/disks/table/disks/DisksTableRow.tsx
  • src/views/virtualmachinesinstance/details/tabs/disks/table/disks/disksTableDefinition.tsx
  • src/views/vmnetworks/list/VMNetworkList.tsx
  • src/views/vmnetworks/list/VMNetworkOtherTypesList.tsx
  • src/views/vmnetworks/list/components/VMNetworkOtherRow.tsx
  • src/views/vmnetworks/list/components/VMNetworkRow.tsx
  • src/views/vmnetworks/list/hooks/useVMNetworkColumns.tsx
  • src/views/vmnetworks/list/vmNetworkListDefinition.tsx
  • src/views/vmnetworks/list/vmNetworkOtherTypesListDefinition.tsx
💤 Files with no reviewable changes (10)
  • src/views/vmnetworks/list/hooks/useVMNetworkColumns.tsx
  • src/views/catalog/templatescatalog/hooks/useTemplatesCatalogColumns.ts
  • src/views/virtualmachinesinstance/details/tabs/disks/hooks/useDisksTableColumns.ts
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/components/NodeRow.scss
  • src/views/catalog/templatescatalog/components/TemplatesCatalogRow.tsx
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/utils/hooks/useNodesColumns.ts
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/components/NodeRow.tsx
  • src/views/vmnetworks/list/components/VMNetworkOtherRow.tsx
  • src/views/vmnetworks/list/components/VMNetworkRow.tsx
  • src/views/virtualmachinesinstance/details/tabs/disks/table/disks/DisksTableRow.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
  • src/views/vmnetworks/list/vmNetworkListDefinition.tsx
  • src/views/vmnetworks/list/vmNetworkOtherTypesListDefinition.tsx
  • locales/ko/plugin__kubevirt-plugin.json
  • locales/es/plugin__kubevirt-plugin.json
  • locales/ja/plugin__kubevirt-plugin.json
  • src/views/virtualmachinesinstance/details/tabs/disks/table/disks/DisksTable.tsx

"{{title}}: {{percentage}}%": "{{title}}: {{percentage}}%",
"{{tolerations}} Toleration rules": "{{tolerations}} Toleration rules",
"{{used}} / {{total}} used": "{{used}} / {{total}} used",
"{{used}} cores / {{total}} cores": "{{used}} cores / {{total}} cores",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid hardcoding the pluralized unit here.

If either side is 1, this renders 1 cores, which is a valid input for CPU counts. Prefer a neutral label like {{used}} / {{total}} cores, or pass fully formatted usedLabel / totalLabel strings from the caller.

💬 Safer wording
-  "{{used}} cores / {{total}} cores": "{{used}} cores / {{total}} cores",
+  "{{used}} / {{total}} cores": "{{used}} / {{total}} cores",
📝 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.

Suggested change
"{{used}} cores / {{total}} cores": "{{used}} cores / {{total}} cores",
"{{used}} / {{total}} cores": "{{used}} / {{total}} cores",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@locales/en/plugin__kubevirt-plugin.json` at line 86, The localization string
currently hardcodes the plural "cores" in the key/value "{{used}} cores /
{{total}} cores", which yields incorrect output like "1 cores"; update the
resource so it avoids pluralizing the unit directly—either change the string to
a neutral form such as "{{used}} / {{total}} cores" or accept fully formatted
labels from the caller (e.g., use keys like "{{usedLabel}} / {{totalLabel}}")
and update callers that reference "{{used}} cores / {{total}} cores" to pass
properly pluralized labels (or the neutral form) instead; ensure you update the
JSON key/value and any call-sites that interpolate used/total accordingly.

Comment on lines +5 to +6
availableDatasources: Record<string, V1beta1DataSource>;
availableTemplatesUID: Set<string>;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Make the shared lookup collections sparse/readonly.

Record<string, V1beta1DataSource> says every string key exists, so availableDatasources[someKey] narrows to V1beta1DataSource instead of V1beta1DataSource | undefined. For an availability map, that hides missing-entry cases from the type system. I’d also expose both collections as readonly since consumers should not mutate table context.

🛠️ Suggested typing adjustment
 export type TemplatesCatalogCallbacks = {
-  availableDatasources: Record<string, V1beta1DataSource>;
-  availableTemplatesUID: Set<string>;
+  availableDatasources: Readonly<Partial<Record<string, V1beta1DataSource>>>;
+  availableTemplatesUID: ReadonlySet<string>;
   onTemplateClick: (template: V1Template) => void;
 };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/views/catalog/templatescatalog/types.ts` around lines 5 - 6, The current
types make the lookup collections appear dense and mutable; change
availableDatasources to be sparse and readonly and make availableTemplatesUID
readonly: replace availableDatasources: Record<string, V1beta1DataSource> with
availableDatasources: Readonly<Record<string, V1beta1DataSource | undefined>>
and replace availableTemplatesUID: Set<string> with availableTemplatesUID:
ReadonlySet<string> so callers see possible undefined entries and cannot mutate
the collections.

Signed-off-by: batyana <bnahmias@redhat.com>
@batyana batyana force-pushed the CNV-74280-pr7-simple-tables branch from bc3a818 to 0f44372 Compare March 16, 2026 12:23
@openshift-ci-robot
Copy link
Collaborator

@batyana: This pull request references CNV-74280 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

📝 Description

Summary

  • Migrates 5 simple tables from deprecated VirtualizedTable to KubevirtTable wrapper
  • Part of CNV-74280 table migration effort
  • Tables: VMI DisksTable, VMNetworkList, VMNetworkOtherTypesList, NodesTable, TemplatesCatalogItems

🎥 Demo

Screen.Recording.2026-03-10.at.13.44.26.mov

Summary by CodeRabbit

  • Documentation

  • Added translations (EN, ES, FR, JA, KO, ZH) for table labels and empty-state messages.

  • New Features

  • New table labels for Disks, Nodes, VM Networks, Other VM Networks, and Templates catalog.

  • Templates catalog now receives unfiltered template data.

  • Improved search input accessibility (aria-label).

  • Bug Fixes

  • Clearer "no data" / "no results" messaging and more reliable empty-state behavior across templates, nodes, disks, and VM network views.

Instructions 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 openshift-eng/jira-lifecycle-plugin repository.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/NodesTable.tsx (1)

27-30: Make node-name filtering case-insensitive and trimmed.

Current matching is exact-case substring only, so common inputs like uppercase/lowercase variants won’t match reliably.

Suggested diff
-  const filteredData = useMemo(
-    () => nodesData.filter((nodeData) => nodeData?.name?.includes(searchValue)),
-    [searchValue, nodesData],
-  );
+  const filteredData = useMemo(() => {
+    const normalizedSearch = searchValue.trim().toLowerCase();
+    return nodesData.filter((nodeData) =>
+      nodeData?.name?.toLowerCase().includes(normalizedSearch),
+    );
+  }, [searchValue, nodesData]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/NodesTable.tsx`
around lines 27 - 30, The current filteredData useMemo uses
nodeData?.name?.includes(searchValue) which is case-sensitive and doesn't trim
input; update the predicate to normalize both sides by trimming and lowercasing
(e.g., derive a normalizedSearch = (searchValue || '').trim().toLowerCase() and
compare against (nodeData?.name || '').trim().toLowerCase()) so includes becomes
case-insensitive and ignores surrounding whitespace; keep the same useMemo
dependencies (searchValue, nodesData) and ensure null/undefined names are
handled safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@locales/ko/plugin__kubevirt-plugin.json`:
- Around line 81-84: The listed Korean locale entries are still in English;
replace the English values for the keys "{{used}} cores / {{total}} cores",
"{{used}}/{{total}} {{unit}} used", "{{usedText}} used", and "{{usedValue}}
{{usedUnit}} / {{totalValue}} {{totalUnit}}" with accurate Korean translations
so the Korean locale file contains Korean strings (also update the same English
values at the other reported occurrences: the entries at lines referenced for
678, 1277, 1299, 1300, 1355, 1414, 1670, 1893, and 2229). Ensure placeholder
tokens (e.g. {{used}}, {{total}}, {{unit}}, {{usedText}}, {{usedValue}},
{{usedUnit}}, {{totalValue}}, {{totalUnit}}) are preserved exactly in the
translated values.

In `@locales/zh/plugin__kubevirt-plugin.json`:
- Line 81: The zh locale currently contains English fallbacks (e.g. the key
"{{used}} cores / {{total}} cores") and several new table/empty-state keys;
replace those English values with proper Chinese translations, preserving
interpolation placeholders (e.g. keep "{{used}}" and "{{total}}") and JSON
syntax; specifically update the entry for "{{used}} cores / {{total}} cores" and
the other new table/empty-state keys referenced in the review so they contain
correct Chinese strings, then validate the JSON and run i18n linting to ensure
no missing keys or syntax errors.

In
`@src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/nodesTableDefinition.tsx`:
- Line 134: The getNodeRowId function currently falls back to a non-unique
literal ('unknown-node') which can cause duplicate row IDs; since NodeData.name
is a required property, change getNodeRowId to return node.name directly (remove
the 'unknown-node' fallback) so each row uses the unique required name
(referencing getNodeRowId and NodeData.name).

In `@src/views/vmnetworks/list/VMNetworkOtherTypesList.tsx`:
- Around line 30-31: The hasData prop is set using truthiness (hasData={!!data})
which is true for empty arrays; change the prop to use an emptiness check so
StateHandler gets correct semantics: set hasData={!isEmpty(data)} (import/use
the existing isEmpty helper) in the VMNetworkOtherTypesList component where
StateHandler is rendered, so loading logic that depends on loaded and hasData
works correctly.

---

Nitpick comments:
In
`@src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/NodesTable.tsx`:
- Around line 27-30: The current filteredData useMemo uses
nodeData?.name?.includes(searchValue) which is case-sensitive and doesn't trim
input; update the predicate to normalize both sides by trimming and lowercasing
(e.g., derive a normalizedSearch = (searchValue || '').trim().toLowerCase() and
compare against (nodeData?.name || '').trim().toLowerCase()) so includes becomes
case-insensitive and ignores surrounding whitespace; keep the same useMemo
dependencies (searchValue, nodesData) and ensure null/undefined names are
handled safely.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 991baa91-0e91-4eb3-83fe-3125825207da

📥 Commits

Reviewing files that changed from the base of the PR and between bc3a818 and 0f44372.

📒 Files selected for processing (29)
  • locales/en/plugin__kubevirt-plugin.json
  • locales/es/plugin__kubevirt-plugin.json
  • locales/fr/plugin__kubevirt-plugin.json
  • locales/ja/plugin__kubevirt-plugin.json
  • locales/ko/plugin__kubevirt-plugin.json
  • locales/zh/plugin__kubevirt-plugin.json
  • src/views/catalog/templatescatalog/TemplatesCatalog.tsx
  • src/views/catalog/templatescatalog/components/TemplatesCatalogItems.tsx
  • src/views/catalog/templatescatalog/components/TemplatesCatalogRow.tsx
  • src/views/catalog/templatescatalog/hooks/useTemplatesCatalogColumns.ts
  • src/views/catalog/templatescatalog/templatesCatalogCells.tsx
  • src/views/catalog/templatescatalog/templatesCatalogTableDefinition.tsx
  • src/views/catalog/templatescatalog/types.ts
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/NodesTable.tsx
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/components/NodeRow.scss
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/components/NodeRow.tsx
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/nodesTableDefinition.tsx
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/utils/hooks/useNodesColumns.ts
  • src/views/virtualmachinesinstance/details/tabs/disks/hooks/useDisksTableColumns.ts
  • src/views/virtualmachinesinstance/details/tabs/disks/table/disks/DisksTable.tsx
  • src/views/virtualmachinesinstance/details/tabs/disks/table/disks/DisksTableRow.tsx
  • src/views/virtualmachinesinstance/details/tabs/disks/table/disks/disksTableDefinition.tsx
  • src/views/vmnetworks/list/VMNetworkList.tsx
  • src/views/vmnetworks/list/VMNetworkOtherTypesList.tsx
  • src/views/vmnetworks/list/components/VMNetworkOtherRow.tsx
  • src/views/vmnetworks/list/components/VMNetworkRow.tsx
  • src/views/vmnetworks/list/hooks/useVMNetworkColumns.tsx
  • src/views/vmnetworks/list/vmNetworkListDefinition.tsx
  • src/views/vmnetworks/list/vmNetworkOtherTypesListDefinition.tsx
💤 Files with no reviewable changes (10)
  • src/views/vmnetworks/list/hooks/useVMNetworkColumns.tsx
  • src/views/vmnetworks/list/components/VMNetworkRow.tsx
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/components/NodeRow.scss
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/utils/hooks/useNodesColumns.ts
  • src/views/catalog/templatescatalog/components/TemplatesCatalogRow.tsx
  • src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/components/NodeRow.tsx
  • src/views/catalog/templatescatalog/hooks/useTemplatesCatalogColumns.ts
  • src/views/virtualmachinesinstance/details/tabs/disks/hooks/useDisksTableColumns.ts
  • src/views/virtualmachinesinstance/details/tabs/disks/table/disks/DisksTableRow.tsx
  • src/views/vmnetworks/list/components/VMNetworkOtherRow.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
  • src/views/vmnetworks/list/vmNetworkListDefinition.tsx
  • locales/es/plugin__kubevirt-plugin.json
  • src/views/vmnetworks/list/vmNetworkOtherTypesListDefinition.tsx
  • src/views/catalog/templatescatalog/templatesCatalogCells.tsx
  • locales/ja/plugin__kubevirt-plugin.json
  • locales/fr/plugin__kubevirt-plugin.json

Comment on lines +81 to +84
"{{used}} cores / {{total}} cores": "{{used}} cores / {{total}} cores",
"{{used}}/{{total}} {{unit}} used": "{{used}}/{{total}} {{unit}} used",
"{{usedText}} used": "{{usedText}} used",
"{{usedValue}} {{usedUnit}} / {{totalValue}} {{totalUnit}}": "{{usedValue}} {{usedUnit}} / {{totalValue}} {{totalUnit}}",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Translate newly added Korean locale entries (currently English).

At Line 81, Line 84, Line 678, Line 1277, Line 1299, Line 1300, Line 1355, Line 1414, Line 1670, Line 1893, and Line 2229, values are still English. This will surface mixed-language UI in Korean for table labels and empty states.

Suggested patch
-  "{{used}} cores / {{total}} cores": "{{used}} cores / {{total}} cores",
+  "{{used}} cores / {{total}} cores": "{{used}} 코어 / {{total}} 코어",

-  "{{usedValue}} {{usedUnit}} / {{totalValue}} {{totalUnit}}": "{{usedValue}} {{usedUnit}} / {{totalValue}} {{totalUnit}}",
+  "{{usedValue}} {{usedUnit}} / {{totalValue}} {{totalUnit}}": "{{usedValue}} {{usedUnit}} / {{totalValue}} {{totalUnit}}",

-  "Disks table": "Disks table",
+  "Disks table": "디스크 테이블",

-  "No disks found": "No disks found",
+  "No disks found": "디스크를 찾을 수 없음",

-  "No nodes found": "No nodes found",
+  "No nodes found": "노드를 찾을 수 없음",

-  "No nodes match the search criteria": "No nodes match the search criteria",
+  "No nodes match the search criteria": "검색 기준과 일치하는 노드가 없음",

-  "Nodes table": "Nodes table",
+  "Nodes table": "노드 테이블",

-  "Other VM Networks table": "Other VM Networks table",
+  "Other VM Networks table": "기타 VM 네트워크 테이블",

-  "Select node {{name}}": "Select node {{name}}",
+  "Select node {{name}}": "노드 {{name}} 선택",

-  "Templates catalog table": "Templates catalog table",
+  "Templates catalog table": "템플릿 카탈로그 테이블",

-  "VM Networks table": "VM Networks table",
+  "VM Networks table": "VM 네트워크 테이블",

Also applies to: 678-678, 1277-1277, 1299-1300, 1355-1355, 1414-1414, 1670-1670, 1893-1893, 2229-2229

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@locales/ko/plugin__kubevirt-plugin.json` around lines 81 - 84, The listed
Korean locale entries are still in English; replace the English values for the
keys "{{used}} cores / {{total}} cores", "{{used}}/{{total}} {{unit}} used",
"{{usedText}} used", and "{{usedValue}} {{usedUnit}} / {{totalValue}}
{{totalUnit}}" with accurate Korean translations so the Korean locale file
contains Korean strings (also update the same English values at the other
reported occurrences: the entries at lines referenced for 678, 1277, 1299, 1300,
1355, 1414, 1670, 1893, and 2229). Ensure placeholder tokens (e.g. {{used}},
{{total}}, {{unit}}, {{usedText}}, {{usedValue}}, {{usedUnit}}, {{totalValue}},
{{totalUnit}}) are preserved exactly in the translated values.

"{{title}}: {{percentage}}%": "{{title}}: {{percentage}}%",
"{{tolerations}} Toleration rules": "{{tolerations}} 容限规则",
"{{used}} / {{total}} used": "{{used}} / {{total}} used",
"{{used}} cores / {{total}} cores": "{{used}} cores / {{total}} cores",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Localize new Chinese entries instead of shipping English fallbacks.

Line 81 and the new table/empty-state keys (Lines 678, 1277, 1299-1300, 1355, 1414, 1670, 1893, 2229) are still English in the zh locale. This degrades i18n quality for Chinese users.

Suggested zh translations
-  "{{used}} cores / {{total}} cores": "{{used}} cores / {{total}} cores",
+  "{{used}} cores / {{total}} cores": "{{used}} 核 / {{total}} 核",

-  "Disks table": "Disks table",
+  "Disks table": "磁盘表",

-  "No disks found": "No disks found",
+  "No disks found": "未找到磁盘",

-  "No nodes found": "No nodes found",
+  "No nodes found": "未找到节点",

-  "No nodes match the search criteria": "No nodes match the search criteria",
+  "No nodes match the search criteria": "没有符合搜索条件的节点",

-  "Nodes table": "Nodes table",
+  "Nodes table": "节点表",

-  "Other VM Networks table": "Other VM Networks table",
+  "Other VM Networks table": "其他虚拟机网络表",

-  "Select node {{name}}": "Select node {{name}}",
+  "Select node {{name}}": "选择节点 {{name}}",

-  "Templates catalog table": "Templates catalog table",
+  "Templates catalog table": "模板目录表",

-  "VM Networks table": "VM Networks table",
+  "VM Networks table": "虚拟机网络表",

Also applies to: 678-678, 1277-1277, 1299-1300, 1355-1355, 1414-1414, 1670-1670, 1893-1893, 2229-2229

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@locales/zh/plugin__kubevirt-plugin.json` at line 81, The zh locale currently
contains English fallbacks (e.g. the key "{{used}} cores / {{total}} cores") and
several new table/empty-state keys; replace those English values with proper
Chinese translations, preserving interpolation placeholders (e.g. keep
"{{used}}" and "{{total}}") and JSON syntax; specifically update the entry for
"{{used}} cores / {{total}} cores" and the other new table/empty-state keys
referenced in the review so they contain correct Chinese strings, then validate
the JSON and run i18n linting to ensure no missing keys or syntax errors.

},
];

export const getNodeRowId = (node: NodeData): string => node.name ?? 'unknown-node';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid non-unique fallback row IDs.

Using 'unknown-node' as a fallback can collapse multiple rows into the same key and break selection/render behavior. Since NodeData.name is required (src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/utils/types.ts, Line 7-19), prefer returning node.name directly.

Suggested diff
-export const getNodeRowId = (node: NodeData): string => node.name ?? 'unknown-node';
+export const getNodeRowId = (node: NodeData): string => node.name;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/views/virtualmachines/actions/components/VirtualMachineComputeMigration/components/NodesTable/nodesTableDefinition.tsx`
at line 134, The getNodeRowId function currently falls back to a non-unique
literal ('unknown-node') which can cause duplicate row IDs; since NodeData.name
is a required property, change getNodeRowId to return node.name directly (remove
the 'unknown-node' fallback) so each row uses the unique required name
(referencing getNodeRowId and NodeData.name).

Comment on lines 30 to +31
<StateHandler error={error} hasData={!!data} loaded={loaded} showSkeletonLoading withBullseye>
{isEmpty(data) ? (
{loaded && isEmpty(data) ? (
Copy link

@coderabbitai coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Verify how StateHandler uses `hasData` in its internal branching.
fd 'StateHandler.tsx' -t f | xargs -r rg -n -C4 '\bhasData\b|loaded|error|showSkeletonLoading'

# Compare usage patterns in peer list views after table migration.
rg -n -C2 '<StateHandler[^>]*hasData=' src/views -g '*.tsx'

Repository: kubevirt-ui/kubevirt-plugin

Length of output: 4207


🏁 Script executed:

# Get full StateHandler implementation
fd 'StateHandler.tsx' -t f | xargs -r cat | head -100

Repository: kubevirt-ui/kubevirt-plugin

Length of output: 1726


🏁 Script executed:

# Check VMNetworkOtherTypesList data type and where isEmpty is imported from
cat -n src/views/vmnetworks/list/VMNetworkOtherTypesList.tsx | head -50

Repository: kubevirt-ui/kubevirt-plugin

Length of output: 2340


🏁 Script executed:

# Check how isEmpty is defined - is it lodash or custom?
rg -n "^import.*isEmpty" src/views/vmnetworks/list/VMNetworkOtherTypesList.tsx

Repository: kubevirt-ui/kubevirt-plugin

Length of output: 127


🏁 Script executed:

# Find useListPageFilter definition to understand its return type
rg -n "export.*useListPageFilter|function useListPageFilter" -A 3

Repository: kubevirt-ui/kubevirt-plugin

Length of output: 186


Use emptiness check for hasData instead of truthiness check.

Line 30 uses hasData={!!data}, which evaluates to true even when data is an empty array. This causes StateHandler to skip its loading condition (!loaded && !hasData), rendering table children during the initial load instead of showing the loading spinner.

Use hasData={!isEmpty(data)} to correctly indicate whether actual data exists.

Suggested fix
-    <StateHandler error={error} hasData={!!data} loaded={loaded} showSkeletonLoading withBullseye>
+    <StateHandler
+      error={error}
+      hasData={!isEmpty(data)}
+      loaded={loaded}
+      showSkeletonLoading
+      withBullseye
+    >
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/views/vmnetworks/list/VMNetworkOtherTypesList.tsx` around lines 30 - 31,
The hasData prop is set using truthiness (hasData={!!data}) which is true for
empty arrays; change the prop to use an emptiness check so StateHandler gets
correct semantics: set hasData={!isEmpty(data)} (import/use the existing isEmpty
helper) in the VMNetworkOtherTypesList component where StateHandler is rendered,
so loading logic that depends on loaded and hasData works correctly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasData is actually a redundant prop in the StateHandler, it does the exact same thing as loaded, I will remove the prop later

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamviktora, thanks for the clarification! That makes sense — I'll disregard my earlier concern then.


📝 Learnings were identified and not saved (knowledge base disabled). Enable


if (namespace) {
return (
<ResourceLink groupVersionKind={modelToGroupVersionKind(NamespaceModel)} name={namespace} />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing the wrapping span with data-test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants