Skip to content

Conversation

@rhamilto
Copy link
Member

@rhamilto rhamilto commented Sep 12, 2025

Duplicates changes also in #15489

This PR updates list views for the following Workloads:

  • public/components/hpa.tsx
  • public/components/job.tsx
  • public/components/pod.jsx
  • public/components/replicaset.jsx
  • public/components/replication-controller.jsx
  • public/components/stateful-set.tsx

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 12, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 12, 2025

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

Details

In response to this:

Includes changes from #15469, which should merge first.

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 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 Sep 12, 2025
@openshift-ci openshift-ci bot added component/core Related to console core functionality kind/cypress Related to Cypress e2e integration testing approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 12, 2025
@openshift-ci openshift-ci bot added the kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated label Sep 12, 2025
@rhamilto rhamilto force-pushed the CONSOLE-4720 branch 8 times, most recently from 801b835 to 9e43c49 Compare September 17, 2025 19:55
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 17, 2025
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 18, 2025
@rhamilto rhamilto force-pushed the CONSOLE-4720 branch 2 times, most recently from 061cbe5 to 30942f0 Compare September 18, 2025 15:33
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 21, 2025
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 30, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 30, 2025

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

Details

In response to this:

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
Copy link
Contributor

openshift-ci-robot commented Sep 30, 2025

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

Details

In response to this:

This PR updates list views for the following Workloads:

  • public/components/hpa.tsx
  • public/components/job.tsx
  • public/components/pod.jsx
  • public/components/replicaset.jsx
  • public/components/replication-controller.jsx
  • public/components/stateful-set.tsx

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 openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2025
@rhamilto
Copy link
Member Author

/label tide/merge-method-squash

@openshift-ci openshift-ci bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Oct 14, 2025
@rhamilto
Copy link
Member Author

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 14, 2025
@rhamilto
Copy link
Member Author

3905e76 restores the namespace column help text that was inadvertently omitted

Screenshot 2025-10-14 at 11 07 54 AM

Copy link
Member

@TheRealJon TheRealJon left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2025
@sferich888
Copy link

/label px-approved

@openshift-ci openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Oct 14, 2025
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2025
@TheRealJon
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2025
@rhamilto
Copy link
Member Author

/retest

@yapei
Copy link
Contributor

yapei commented Oct 15, 2025

not sure why verified label got removed, tested again and confirmed working well
/verified by @yapei

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Oct 15, 2025
@openshift-ci-robot
Copy link
Contributor

@yapei: This PR has been marked as verified by @yapei.

Details

In response to this:

not sure why verified label got removed, tested again and confirmed working well
/verified by @yapei

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

@jseseCCS jseseCCS left a comment

Choose a reason for hiding this comment

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

Just a few comments but approved! will add approving label now.

>
{!columnLayout?.showNamespaceOverride &&
t('public~The namespace column is only shown when in "All projects"')}
{!columnLayout?.showNamespaceOverride && <NamespaceColumnHelpText />}

Choose a reason for hiding this comment

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

Please confirm that renders same visible message shown here: “The namespace column is only shown when in ‘All projects’.” If wording/cap differs, I should review it for user-facing consistency/doc alignment.

Copy link
Member Author

Choose a reason for hiding this comment

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

The wording has not changed.

kinds={['Pods']}
ListComponent={PodList}
rowFilters={podFilters}
hideColumnManagement={true}

Choose a reason for hiding this comment

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

Please confirm that setting hideColumnManagement and omitFilterToolbar removes those controls from CronJob list page. If any visible UI elements (column selector, filter bar) differ from prior behavior, I should review for user-facing consistency/doc alignment.

Copy link
Member Author

Choose a reason for hiding this comment

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

No differences.

}}
kinds={['Jobs']}
ListComponent={JobsList}
hideColumnManagement={true}

Choose a reason for hiding this comment

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

ditto above: Confirm hideColumnManagement and omitFilterToolbar...

Copy link
Member Author

Choose a reason for hiding this comment

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

No differences.

<span className="co-resource-item">
<ResourceLink
kind="CustomResourceDefinition"
groupVersionKind={getGroupVersionKindForModel(CustomResourceDefinitionModel)}

Choose a reason for hiding this comment

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

Please confirm that replacing static "CustomResourceDefinition" string with {kind} and {getGroupVersionKindForModel(CustomResourceDefinitionModel)} doesn’t change any visible kind labels/kebab-menu behavior in CustomResourceDefinition list/detail views. If user-facing resource name or actions differ, I should review for consistency/doc alignment.

Copy link
Member Author

Choose a reason for hiding this comment

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

No differences.

</TableData>
</>
);
const rowCells = {

Choose a reason for hiding this comment

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

Please confirm that refactoring HPA list rows to use rowCells + ResourceLink doesn’t change visible table behavior/row actions. If any column layout or kebab-menu options differ, I should review for user-facing consistency/doc alignment.

}
describe(kind, () => {
const name = `${testName}-${_.kebabCase(kind)}`;
const isDataViewResource = dataViewResources.has(kind);

Choose a reason for hiding this comment

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

Please confirm that adding isDataViewResource logic doesn’t affect expected UI behavior/labeling in resource CRUD tests. If any test-visible elements (resource names, menu options, form fields) differ between DataView/non-DataView resources, I should review for user-facing consistency/doc alignment.

Copy link
Member Author

Choose a reason for hiding this comment

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

No differences.

listPage.filter.byName(text);
listPage.rows.countShouldBeWithin(1, 3);
listPage.rows.clickRowByName(text);
listPage.dvFilter.byName(text);

Choose a reason for hiding this comment

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

Please confirm that switching from filter/rows to dvFilter/dvRows doesn’t change user-visible filtering/table behavior on namespace CRUD pages. If any list-page controls, column visibility, selection options differ, I should review for user-facing consistency/doc alignment.

Copy link
Member Author

Choose a reason for hiding this comment

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

No differences.

cy.get('@filterDropdownToggleButton').click();
},
},
dvFilter: {

Choose a reason for hiding this comment

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

Please confirm that new dvFilter, dvRows helpers reflect same user-facing list interactions as prior filter/rows functions. If any visible list controls, filter options behave differently, I should review for user-facing consistency/doc alignment.

Copy link
Member Author

Choose a reason for hiding this comment

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

No differences.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 15, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jseseCCS, rhamilto, TheRealJon

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

The pull request process is described 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

@jseseCCS
Copy link

/label docs-approved

@openshift-ci openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Oct 15, 2025
@rhamilto
Copy link
Member Author

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 15, 2025

@rhamilto: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-gcp-console 0ea120c link true /test e2e-gcp-console

Full PR test history. Your PR dashboard.

Details

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.

@rhamilto
Copy link
Member Author

/retest

@rhamilto
Copy link
Member Author

Changes are included in #15560, which will merge first.

@rhamilto rhamilto closed this Oct 15, 2025
@logonoff logonoff deleted the CONSOLE-4720 branch October 15, 2025 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality docs-approved Signifies that Docs has signed off on this PR jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. kind/cypress Related to Cypress e2e integration testing kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants