frontend/packages/console-app/src/components/volume-snapshot/volume-snapshot-content.tsx (1)

51-53: Consider normalizing restoreSize before humanizing for consistency

Here you call humanizeBinaryBytes(size).string directly on obj.status?.restoreSize. In volume-snapshot.tsx, size is first normalized with convertToBaseValue before humanization. Aligning both places (e.g., via a shared helper or the same pre-normalization step) would avoid subtle unit-handling differences and keep the UX consistent between snapshots and snapshot contents.

frontend/packages/console-app/src/components/volume-snapshot/volume-snapshot.tsx (2)

63-80: Make getDataViewRows resilient to missing rowData

Right now you destructure rowData: { hideSnapshotContentColumn } directly from each element. This assumes rowData is always present, which is true given the current customRowData usage, but it makes the helper fragile to future reuse. A small tweak to handle an optional rowData (e.g., const hideSnapshotContentColumn = rowData?.hideSnapshotContentColumn;) would make this safer without changing behavior today.

---

227-275: Confirm status filter values stay aligned with volumeSnapshotStatus

The status filter options (Ready, Pending, Error) are hard-coded, and matchesAdditionalFilters checks volumeSnapshotStatus(resource) against these strings. This is fine as long as volumeSnapshotStatus continues to return the same labels, but it’s a silent coupling. Worth double‑checking they match today and considering a shared source of truth if more statuses are added later.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

frontend/public/components/persistent-volume-claim.tsx (2)

103-174: Avoid using hooks directly inside getDataViewRows utility

getDataViewRows calls useTranslation and useSelector while explicitly disabling react-hooks/rules-of-hooks. This is brittle: if ConsoleDataView ever calls this callback conditionally or in a non-render context, hook ordering can break, and static analysis can’t protect you.

Consider moving the hook usage into a dedicated hook that returns a memoized row-mapping function, e.g. usePVCDataViewRows(): GetDataViewRows<PersistentVolumeClaimKind>, and then pass that from PersistentVolumeClaimList to ConsoleDataView. That keeps hooks at the top level of a React hook/component and lets you re‑enable the eslint rule.

---

584-588: Align PersistentVolumeClaimListProps['data'] type with what GetDataViewRows expects

PersistentVolumeClaimListProps currently types data as PersistentVolumeClaimKind[], but getDataViewRows treats each element as an object with an obj property. To avoid type drift and ensure callers pass the correct row shape, it’s safer to derive the element type from GetDataViewRows:

-type PersistentVolumeClaimListProps = {
-  data: PersistentVolumeClaimKind[];
+type PersistentVolumeClaimListProps = {
+  data: Parameters<GetDataViewRows<PersistentVolumeClaimKind>>[0];
   loaded: boolean;
   loadError: unknown;
 };

This keeps the list’s public props in sync with the DataView row contract even if the underlying row type changes.

frontend/packages/console-app/src/components/volume-snapshot/volume-snapshot-content.tsx (1)

45-53: Consider normalizing restoreSize before humanizing

In getDataViewRows, sizeMetrics calls humanizeBinaryBytes(size) directly, whereas the VolumeSnapshot table converts quantities via convertToBaseValue first. If status.restoreSize is a quantity string, mirroring that pattern here (humanizeBinaryBytes(convertToBaseValue(size))) would keep units handling consistent and more robust.

frontend/packages/console-app/src/components/volume-snapshot/volume-snapshot-class.tsx (1)

34-39: Use computed property name for the default-class annotation key

In isDefaultSnapshotClass, the default annotations object uses a literal key defaultSnapshotClassAnnotation:

getAnnotations(volumeSnapshotClass, { defaultSnapshotClassAnnotation: 'false' })[defaultSnapshotClassAnnotation]

Using a computed key keeps the constant and the actual annotation key in sync and makes the fallback clearer:

-  getAnnotations(volumeSnapshotClass, { defaultSnapshotClassAnnotation: 'false' })[
-    defaultSnapshotClassAnnotation
-  ] === 'true';
+  getAnnotations(volumeSnapshotClass, { [defaultSnapshotClassAnnotation]: 'false' })[
+    defaultSnapshotClassAnnotation
+  ] === 'true';

This avoids any future drift if the annotation name changes.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

frontend/public/components/persistent-volume-claim.tsx (1)

510-533: Confirm initPath behavior when namespace is undefined

initPath currently falls back to the default namespace:

const initPath = `/k8s/ns/${namespace || 'default'}/persistentvolumeclaims/`;

If PersistentVolumeClaimsPage is ever used without an explicit namespace, this will route create links into the default namespace rather than an all-namespaces/cluster-scoped path (if that was the previous behavior).

If the intended behavior is “all namespaces” when namespace is unset, consider something like:

const initPath = namespace
  ? `/k8s/ns/${namespace}/persistentvolumeclaims/`
  : '/k8s/all-namespaces/persistentvolumeclaims/';

Please double-check this against existing PVC pages to avoid changing navigation semantics unexpectedly.

frontend/packages/console-app/src/components/volume-snapshot/volume-snapshot-content.tsx (1)

35-43: Row/column mapping is sound; consider aligning size formatting with VolumeSnapshot

The data-view rows correctly map all tableColumnInfo ids to cells, with sensible fallbacks (DASH) and appropriate use of ResourceLink, Status, Timestamp, and LazyActionMenu. For consistency with VolumeSnapshot sizing, you may want to confirm that status.restoreSize is already in base bytes; if not, consider going through convertToBaseValue before humanizeBinaryBytes to avoid any subtle formatting differences.

If restoreSize is not guaranteed to be base bytes, please double‑check current usage of humanizeBinaryBytes with similar fields elsewhere in the codebase to decide whether a convertToBaseValue step is needed here.

Also applies to: 45-103, 105-156

frontend/packages/console-app/src/components/volume-snapshot/volume-snapshot.tsx (2)

63-146: Guard against future column/rowCells drift in getDataViewRows

The current implementation assumes every columns entry has a corresponding rowCells[id], and uses rowCells[id].disabled in the filter predicate. That’s fine today because both are derived from the same tableColumnInfo, but it’s brittle to future changes (e.g., adding a column but forgetting to extend rowCells would cause a runtime error).

You can make this more robust with optional chaining and a default:

-    return columns
-      .filter(({ id }) => !rowCells[id].disabled)
-      .map(({ id }) => {
-        const cell = rowCells[id]?.cell || DASH;
-        const props = rowCells[id]?.props || undefined;
+    return columns
+      .filter(({ id }) => !rowCells[id]?.disabled)
+      .map(({ id }) => {
+        const cell = rowCells[id]?.cell || DASH;
+        const props = rowCells[id]?.props || undefined;
         return {
           id,
           props,
           cell,
         };
       });

---

335-369: PVC page behavior preserved; consider making ns optional in the type

VolumeSnapshotPVCPage preserves the existing behavior of preferring the ns prop and falling back to params.ns, and reuses VolumeSnapshotTable so the new data‑view behavior is applied consistently.

Given that the component already handles a missing ns by reading from route params, you may want to reflect that in the type by making ns optional for accuracy:

-type VolumeSnapshotPVCPage = {
-  obj: PersistentVolumeClaimKind;
-  ns: string;
-};
+type VolumeSnapshotPVCPage = {
+  obj: PersistentVolumeClaimKind;
+  ns?: string;
+};

This isn’t required for runtime correctness but better documents the actual call pattern.

Also applies to: 382-385

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

frontend/public/components/persistent-volume.tsx (1)

55-113: Rules of Hooks violation remains unresolved.

The getDataViewRows function still calls useTranslation() (line 57) despite not being a React component or custom hook. This was flagged in a previous review with suggested fixes. Please address this critical issue before merging.

frontend/public/components/persistent-volume-claim.tsx (1)

104-175: Rules of Hooks violation remains unresolved.

The getDataViewRows function calls both useTranslation() (line 106) and useSelector() (line 107) despite not being a React component or custom hook. This was flagged in a previous review with a detailed refactoring suggestion. Please address this critical issue before merging.

frontend/public/components/storage-class.tsx (1)

252-260: Improve type safety in type definitions.

Two type-related improvements:

1. Line 259: obj: any should use a more specific type like StorageClassResourceKind for better type safety.
2. Line 255: loadError is declared in StorageClassListProps but not used in the component at line 160. Either use it (e.g., for error handling in ConsoleDataView) or remove it from the type definition.

Apply this diff to improve type safety:

 export type StorageClassDetailsProps = {
-  obj: any;
+  obj: StorageClassResourceKind;
 };

frontend/packages/console-app/src/components/volume-snapshot/volume-snapshot.tsx (2)

63-66: Make row/column mapping resilient to future column changes

getDataViewRows filters with rowCells[id].disabled:

return columns
  .filter(({ id }) => !rowCells[id].disabled)
  .map(({ id }) => {
    const cell = rowCells[id]?.cell || DASH;
    const props = rowCells[id]?.props || undefined;
    ...
  });

This assumes every columns entry has a corresponding rowCells[id]. If a new column is ever added and rowCells isn’t updated, this will throw at runtime before the optional chaining is reached.

A small defensive tweak avoids that brittleness:

-  .filter(({ id }) => !rowCells[id].disabled)
+  .filter(({ id }) => !rowCells[id]?.disabled)

You may also not need the disabled flag on rowCells at all, since useVolumeSnapshotColumns already filters disabled columns at the header level.

Also applies to: 82-96, 105-132, 134-146

---

148-215: Status filter wiring looks good; consider centralizing status values

The status checkbox options and matchesAdditionalFilters logic are consistent and should work as intended, assuming volumeSnapshotStatus() returns "Ready" | "Pending" | "Error" for the cases you care about.

To reduce the chance of divergence if statuses evolve, consider sharing constants (or a small enum/union) between volumeSnapshotStatus and volumeSnapshotStatusFilterOptions rather than hardcoding the strings in two places.

Also applies to: 221-226, 227-276, 279-293

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting