Bug 1981272: When deleting PVC inside PVC page the status in the heading doesn't match the status field #9569

cyril-ui-developer · 2021-07-21T14:36:57Z

openshift-ci · 2021-07-21T14:38:08Z

@cyril-ui-developer: An error was encountered searching for bug 1981272 on the Bugzilla server at https://bugzilla.redhat.com. No known errors were detected, please see the full error message for details.

Full error message.


response code 502 not 200

Please contact an administrator to resolve this issue, then request a bug refresh with /bugzilla refresh.

In response to this:

Bug 1981272: When deleting PVC inside PVC page the status in the heading doesn't match the status field

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/test-infra repository.

cyril-ui-developer · 2021-07-21T14:39:10Z

Before:

After:

spadgett · 2021-07-21T16:12:52Z

...console-app/src/components/volume-snapshot/create-volume-snapshot/create-volume-snapshot.tsx

@@ -105,7 +105,7 @@ const PVCSummary: React.FC<PVCSummaryProps> = ({ persistentVolumeClaim }) => {
      </dd>
      <dt>{t('console-app~Status')}</dt>
      <dd>
-        <PVCStatus pvc={persistentVolumeClaim} />
+        <PVCStatus {...persistentVolumeClaim} />


Why is this change needed? It seems better to pass the PVC object as a single prop.

Why is this change needed? It seems better to pass the PVC object as a single prop.

@spadgett I changed it because the DetailsPage component getResourceStatus wouldn't accept PVCStatus pvc props object -{pvc:obj}. It's expecting {obj}.
However since the getResourceStatus returns string, I will revert it, then find another way to get status for getResourceStatus.

spadgett · 2021-07-21T16:15:24Z

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

-        pvc: obj,
+        kind,
+        pvc,
+        redirectTo: `/k8s/ns/${pvc.metadata.namespace}/${PersistentVolumeClaimModel.plural}`,


Why is redirectTo needed? It looks like we changed the default behavior to go to the list page now.

Avoid building paths manually. Use one of the helpers instead.

Why is redirectTo needed? It looks like we changed the default behavior to go to the list page now.

@spadgett I tested in 4.9, it is not redirecting after delete action.

Avoid building paths manually. Use one of the helpers instead.

Noted. Thanks

Why is redirectTo needed? It looks like we changed the default behavior to go to the list page now.

@spadgett I tested in 4.9, it is not redirecting after delete action.

The PVC is not using the common delete modal that has redirect.

spadgett · 2021-07-21T16:17:17Z

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

@@ -122,7 +124,7 @@ const PVCTableRow = connect(mapStateToProps)(({ obj, index, key, style, metrics
        <ResourceLink kind="Namespace" name={namespace} title={namespace} />
      </TableData>
      <TableData className={tableColumnClasses[2]}>
-        <PVCStatus pvc={obj} />
+        <PVCStatus {...obj} />


Again, I prefer passing the PVC object as a prop. It's unclear why this was needed.

Again, I prefer passing the PVC object as a prop. It's unclear why this was needed.

Will address.

spadgett · 2021-07-21T16:19:53Z

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

 export const PersistentVolumeClaimsDetailsPage = (props) => (
  <DetailsPage
    {...props}
+    getResourceStatus={PVCStatus}


We shouldn't be passing a react component as a value for this prop. getResourceStatus should return a string.

We shouldn't be passing a react component as a value for this prop. getResourceStatus should return a string.

Will address. Thanks.

cyril-ui-developer · 2021-07-26T12:45:13Z

/bugzilla refresh

openshift-ci · 2021-07-26T12:45:17Z

@cyril-ui-developer: This pull request references Bugzilla bug 1981272, which is invalid:

expected the bug to target the "4.9.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

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/test-infra repository.

cyril-ui-developer · 2021-07-26T12:47:09Z

/bugzilla refresh

openshift-ci · 2021-07-26T12:47:18Z

@cyril-ui-developer: This pull request references Bugzilla bug 1981272, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.9.0) matches configured target release for branch (4.9.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @yapei

In response to this:

/bugzilla refresh

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/test-infra repository.

cyril-ui-developer · 2021-07-26T14:21:07Z

@spadgett Updated! Thanks.

spadgett

Thanks @cyril-ui-developer, this looks cleaner

spadgett · 2021-07-26T16:56:42Z

frontend/public/components/modals/delete-pvc-modal.tsx

@@ -72,6 +79,8 @@ const DeletePVCModal = withHandlePromise<DeletePVCModalProps>((props) => {

 export type DeletePVCModalProps = {
  pvc: PersistentVolumeClaimKind;
+  redirectTo?: string;


It looks like this prop is no longer used and can be removed from the types?

It looks like this prop is no longer used and can be removed from the types?

@spadgett Updated.

openshift-ci · 2021-07-27T03:21:37Z

@cyril-ui-developer: This pull request references Bugzilla bug 1981272, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.9.0) matches configured target release for branch (4.9.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @yapei

In response to this:

Bug 1981272: When deleting PVC inside PVC page the status in the heading doesn't match the status field

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/test-infra repository.

openshift-ci · 2021-07-27T03:22:37Z

@cyril-ui-developer: This pull request references Bugzilla bug 1981272, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.9.0) matches configured target release for branch (4.9.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @yapei

In response to this:

[WIP] Bug 1981272: When deleting PVC inside PVC page the status in the heading doesn't match the status field

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/test-infra repository.

spadgett · 2021-07-27T20:44:13Z

frontend/public/components/modals/delete-pvc-modal.tsx

@@ -72,6 +79,7 @@ const DeletePVCModal = withHandlePromise<DeletePVCModalProps>((props) => {

 export type DeletePVCModalProps = {
  pvc: PersistentVolumeClaimKind;
+  kind: K8sKind;


This should always be PersistentVolumeClaimModel right? Do we need to pass kind as a prop?

This should always be PersistentVolumeClaimModel right? Do we need to pass kind as a prop?

Yes, it will. Oh I see! No.
Will use PersistentVolumeClaimModel instead of passing kind for resourceListPathFromModel method.
Thanks for the catch :)

This should always be PersistentVolumeClaimModel right? Do we need to pass kind as a prop?

Updated.

spadgett

/lgtm

Thanks @cyril-ui-developer 👍

openshift-ci · 2021-07-28T16:43:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cyril-ui-developer, spadgett

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~~ [spadgett]

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

spadgett · 2021-07-28T18:38:33Z

/retest

cyril-ui-developer · 2021-07-29T00:47:36Z

/retest

openshift-ci · 2021-07-29T06:27:42Z

@cyril-ui-developer: All pull requests linked via external trackers have merged:

openshift/console#9569

Bugzilla bug 1981272 has been moved to the MODIFIED state.

In response to this:

Bug 1981272: When deleting PVC inside PVC page the status in the heading doesn't match the status field

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/test-infra repository.

openshift-ci bot assigned spadgett Jul 21, 2021

openshift-ci bot requested review from spadgett and zherman0 July 21, 2021 14:39

openshift-ci bot added the component/core Related to console core functionality label Jul 21, 2021

spadgett requested changes Jul 21, 2021

View reviewed changes

cyril-ui-developer force-pushed the deleting-pvc-inside-pvc-page-the-status branch from 209fb97 to ae7c789 Compare July 21, 2021 19:22

openshift-ci bot added the bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. label Jul 26, 2021

openshift-ci bot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Jul 26, 2021

openshift-ci bot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Jul 26, 2021

openshift-ci bot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Jul 26, 2021

openshift-ci bot requested a review from yapei July 26, 2021 12:47

spadgett reviewed Jul 26, 2021

View reviewed changes

cyril-ui-developer force-pushed the deleting-pvc-inside-pvc-page-the-status branch from ae7c789 to d7788d2 Compare July 26, 2021 17:41

cyril-ui-developer changed the title ~~Bug 1981272: When deleting PVC inside PVC page the status in the heading doesn't match the status field~~ [WIP] Bug 1981272: When deleting PVC inside PVC page the status in the heading doesn't match the status field Jul 27, 2021

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 27, 2021

cyril-ui-developer changed the title ~~[WIP] Bug 1981272: When deleting PVC inside PVC page the status in the heading doesn't match the status field~~ Bug 1981272: When deleting PVC inside PVC page the status in the heading doesn't match the status field Jul 27, 2021

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 27, 2021

spadgett reviewed Jul 27, 2021

View reviewed changes

deleting-pvc-inside-pvc-page-the-status

1c0a32b

cyril-ui-developer force-pushed the deleting-pvc-inside-pvc-page-the-status branch from d7788d2 to 1c0a32b Compare July 28, 2021 13:27

spadgett approved these changes Jul 28, 2021

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 28, 2021

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 28, 2021

openshift-merge-robot merged commit 1de1ef3 into openshift:master Jul 29, 2021

spadgett added this to the v4.9 milestone Aug 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1981272: When deleting PVC inside PVC page the status in the heading doesn't match the status field #9569

Bug 1981272: When deleting PVC inside PVC page the status in the heading doesn't match the status field #9569

cyril-ui-developer commented Jul 21, 2021 •

edited

openshift-ci bot commented Jul 21, 2021

cyril-ui-developer commented Jul 21, 2021

spadgett Jul 21, 2021

cyril-ui-developer Jul 21, 2021 •

edited

spadgett Jul 21, 2021

spadgett Jul 21, 2021

cyril-ui-developer Jul 21, 2021

cyril-ui-developer Jul 21, 2021

cyril-ui-developer Jul 21, 2021

spadgett Jul 21, 2021

cyril-ui-developer Jul 21, 2021

spadgett Jul 21, 2021

cyril-ui-developer Jul 21, 2021

cyril-ui-developer commented Jul 26, 2021

openshift-ci bot commented Jul 26, 2021

cyril-ui-developer commented Jul 26, 2021

openshift-ci bot commented Jul 26, 2021

cyril-ui-developer commented Jul 26, 2021

spadgett left a comment

spadgett Jul 26, 2021

cyril-ui-developer Jul 26, 2021

openshift-ci bot commented Jul 27, 2021

openshift-ci bot commented Jul 27, 2021

spadgett Jul 27, 2021

cyril-ui-developer Jul 28, 2021 •

edited

cyril-ui-developer Jul 28, 2021

spadgett left a comment

openshift-ci bot commented Jul 28, 2021

spadgett commented Jul 28, 2021

cyril-ui-developer commented Jul 29, 2021

openshift-ci bot commented Jul 29, 2021

Bug 1981272: When deleting PVC inside PVC page the status in the heading doesn't match the status field #9569

Bug 1981272: When deleting PVC inside PVC page the status in the heading doesn't match the status field #9569

Conversation

cyril-ui-developer commented Jul 21, 2021 • edited

openshift-ci bot commented Jul 21, 2021

cyril-ui-developer commented Jul 21, 2021

Choose a reason for hiding this comment

cyril-ui-developer Jul 21, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyril-ui-developer commented Jul 26, 2021

openshift-ci bot commented Jul 26, 2021

cyril-ui-developer commented Jul 26, 2021

openshift-ci bot commented Jul 26, 2021

cyril-ui-developer commented Jul 26, 2021

spadgett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci bot commented Jul 27, 2021

openshift-ci bot commented Jul 27, 2021

Choose a reason for hiding this comment

cyril-ui-developer Jul 28, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spadgett left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Jul 28, 2021

spadgett commented Jul 28, 2021

cyril-ui-developer commented Jul 29, 2021

openshift-ci bot commented Jul 29, 2021

cyril-ui-developer commented Jul 21, 2021 •

edited

cyril-ui-developer Jul 21, 2021 •

edited

cyril-ui-developer Jul 28, 2021 •

edited