CONSOLE-4580: Refactor PVCActionFactory to hook form. #15078

Mylanos · 2025-05-22T09:43:58Z

Moved away from using the generic kebabFactory for PVC actions that were used inside of the PVC and VolumeSnapshot page. Created hook providers that are being used in these two resource pages to access PVC actions instead. Converted persistent-volume-claim.jsx to TS.
after:

Screen.Recording.2025-05-22.at.11.38.55.mov

Possible tests:

Do all of the actions in PVCs and VolumeSnapshot list and details page behave as expected?
Does the PVC and VolumeSnapshot list and details page contain all of the expected actions?

openshift-ci-robot · 2025-05-22T09:44:02Z

@Mylanos: This pull request references CONSOLE-4580 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.20.0" version, but no target version was set.

In response to this:

Moved away from using the generic kebabFactory for PVC actions that were used inside of the PVC and VolumeSnapshot page. Created hook providers for these two resources that are being used to access PVC actions instead. Converted persistent-volume-claim.jsx to ts.
after:

Screen.Recording.2025-05-22.at.11.38.55.mov

Possible tests:

Do all of the actions in PVCs and VolumeSnapshot behave as expected?

Does the PVC and VolumeSnapshot page contain all of the expected actions?

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.

…sourceActionCreators, replace deprecated K8sKind type, introduce generic type for K8sResourceKind to make the type assertions explicite.

openshift-ci · 2025-05-22T09:45:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Mylanos

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [Mylanos]

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

…tions via hook provider.

openshift-ci-robot · 2025-05-22T09:50:23Z

@Mylanos: This pull request references CONSOLE-4580 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.20.0" version, but no target version was set.

In response to this:

Moved away from using the generic kebabFactory for PVC actions that were used inside of the PVC and VolumeSnapshot page. Created hook providers that are being used in these two resource pages to access PVC actions instead. Converted persistent-volume-claim.jsx to TS.
after:

Screen.Recording.2025-05-22.at.11.38.55.mov

Possible tests:

Do all of the actions in PVCs and VolumeSnapshot behave as expected?

Does the PVC and VolumeSnapshot page contain all of the expected actions?

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-robot · 2025-05-22T09:53:10Z

@Mylanos: This pull request references CONSOLE-4580 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.20.0" version, but no target version was set.

In response to this:

Moved away from using the generic kebabFactory for PVC actions that were used inside of the PVC and VolumeSnapshot page. Created hook providers that are being used in these two resource pages to access PVC actions instead. Converted persistent-volume-claim.jsx to TS.
after:

Screen.Recording.2025-05-22.at.11.38.55.mov

Possible tests:

Do all of the actions in PVCs and VolumeSnapshot list and details page behave as expected?

Does the PVC and VolumeSnapshot list and details page contain all of the expected actions?

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-robot · 2025-05-22T09:53:47Z

@Mylanos: This pull request references CONSOLE-4580 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.20.0" version, but no target version was set.

In response to this:

Moved away from using the generic kebabFactory for PVC actions that were used inside of the PVC and VolumeSnapshot page. Created hook providers that are being used in these two resource pages to access PVC actions instead. Converted persistent-volume-claim.jsx to TS.
after:

Screen.Recording.2025-05-22.at.11.38.55.mov

Possible tests:

Do all of the actions in PVCs and VolumeSnapshot list and details page behave as expected?

Does the PVC and VolumeSnapshot list and details page contain all of the expected actions?

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.

…s, convert persistent-volume-claim.jsx to persistent-volume-claim.tsx, update i18n

…ourceProvider, consume actions from ActionsResourceProvider inside VolumeSnapshots, update i18n, remove PVC and VolumeSnapshot actions from kebabFactory.

Mylanos · 2025-05-26T08:47:20Z

/retest

openshift-ci · 2025-05-26T13:03:20Z

@Mylanos: all tests passed!

Full PR test history. Your PR dashboard.

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

Mylanos · 2025-05-27T15:04:09Z

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

-
-  const [columns] = useActiveColumns({
-    columns: getTableColumns(rowData.customData?.disableItems),
+  const columns = getTableColumns(t, props.rowData.customData.disableItems);


we should make the getTableColumns an inner function to have the t available through the closure. Or explore utilization of useMemo to memoize the columns.

The getTableColumns is used at multiple places - FilteredSnapshotTable and VolumeSnapshotTable, these components don't even share a shared parent, so passing it via props from parent is also not an option. Moving it inside of the both components won't by as DRY as having the function separated in the global scope, so I would opt in for keeping it in the scope outside the components @vojtechszocs

frontend/packages/console-app/src/actions/creators/pvc-factory.ts

Mylanos · 2025-05-27T15:30:33Z

frontend/public/actions/ui.ts

@@ -81,7 +81,7 @@ export type NodeMetrics = {
 };

 export type PVCMetrics = {
-  usedCapacity: MetricValuesByName;
+  usedCapacity?: MetricValuesByName;


This has to be optional, as the pvcMetric fallback value is {}, which causes type errors when dispatching the created pvcMetric. See code.

Mylanos · 2025-05-27T15:42:47Z

frontend/public/components/persistent-volume-claim.tsx

@@ -186,7 +174,7 @@ const Details_ = ({ flags, obj: pvc }) => {
  const totalCapacityMetric = convertToBaseValue(storage);
  const totalRequestMetric = convertToBaseValue(requestedStorage);
  const usedMetrics = response?.data?.result?.[0]?.value?.[1];


fallback value to 0, to never have an empty value.

As discussed, having no fallback value has not caused issues in the past, also the added cast to Number inside of the availableMetrics metrics is protected by fallback value. So I will opt in for keeping the changes, only replacing the casting part from the parseInt() to Number()

frontend/public/components/persistent-volume-claim.tsx

openshift-merge-robot · 2025-05-28T09:11:04Z

PR needs rebase.

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 kubernetes-sigs/prow repository.

Mylanos · 2025-05-28T14:06:19Z

Review comments above are made on behalf of @vojtechszocs, @jon.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 22, 2025

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 22, 2025

CONSOLE-4580: use typescript Record type for the key value list of Re…

7fde975

…sourceActionCreators, replace deprecated K8sKind type, introduce generic type for K8sResourceKind to make the type assertions explicite.

openshift-ci bot requested review from cajieh and rhamilto May 22, 2025 09:45

openshift-ci bot added the component/core Related to console core functionality label May 22, 2025

openshift-ci bot added kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 22, 2025

CONSOLE-4580: convert PVCActionFactory to hook form, expose the PVCAc…

d8052aa

…tions via hook provider.

Mylanos force-pushed the PVCActionFactory-refactor branch from 3910a30 to 2c04374 Compare May 22, 2025 09:49

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 22, 2025

Mylanos added 2 commits May 22, 2025 13:06

CONSOLE-4580: Consume actions from ActionsResourceProvider inside PVC…

09a10d9

…s, convert persistent-volume-claim.jsx to persistent-volume-claim.tsx, update i18n

CONSOLE-4580: create volume snapshot action factory and its ActionRes…

40e05c5

…ourceProvider, consume actions from ActionsResourceProvider inside VolumeSnapshots, update i18n, remove PVC and VolumeSnapshot actions from kebabFactory.

Mylanos force-pushed the PVCActionFactory-refactor branch from 2c04374 to 40e05c5 Compare May 22, 2025 11:06

Mylanos commented May 27, 2025

View reviewed changes

frontend/packages/console-app/src/actions/creators/pvc-factory.ts Show resolved Hide resolved

Mylanos commented May 27, 2025

View reviewed changes

frontend/public/components/persistent-volume-claim.tsx Show resolved Hide resolved

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 28, 2025

CONSOLE-4580: Refactor PVCActionFactory to hook form. #15078

Are you sure you want to change the base?

CONSOLE-4580: Refactor PVCActionFactory to hook form. #15078

Uh oh!

Conversation

Mylanos commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented May 22, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented May 22, 2025

Uh oh!

openshift-ci-robot commented May 22, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented May 22, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented May 22, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mylanos commented May 26, 2025

Uh oh!

openshift-ci bot commented May 26, 2025

Uh oh!

Mylanos May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mylanos May 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mylanos May 27, 2025

Choose a reason for hiding this comment

Uh oh!

Mylanos May 28, 2025

Choose a reason for hiding this comment

Uh oh!

Mylanos May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mylanos May 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

openshift-merge-robot commented May 28, 2025

Uh oh!

Mylanos commented May 28, 2025

Uh oh!

Uh oh!

Mylanos commented May 22, 2025 •

edited

Loading

openshift-ci-robot commented May 22, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented May 22, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented May 22, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented May 22, 2025 •

edited by openshift-ci bot

Loading

Mylanos May 27, 2025 •

edited

Loading

Mylanos May 27, 2025 •

edited

Loading