OCPBUGS-54248: Fix empty state Create button links#423
Conversation
openshift-ci-robot
added
jira/severity-important
|
@pcbailey: This pull request references Jira Issue OCPBUGS-54248, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn 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. |
WalkthroughThis PR enhances the ChangesList view refactoring
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
openshift-ci
Bot
added
the
approved
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@pcbailey: This pull request references Jira Issue OCPBUGS-54248, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn 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. |
|
@pcbailey: This pull request references Jira Issue OCPBUGS-54248, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/components/ListEmptyState/ListEmptyState.tsx (1)
65-72:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard against
navigate(undefined)in the default create button.
createButtonLinkis optional andcanCreatedefaults totrue; if a consumer renders the default button without providingonCreateand also omitscreateButtonLink, the current handler callsnavigate(createButtonLink)wherecreateButtonLinkisundefined(and this won’t necessarily be caught by TS sincestrict/strictNullChecksaren’t enabled in the repo tsconfig files).🛡️ Proposed guard
<Button - onClick={onCreate ? onCreate : () => navigate(createButtonLink)} + onClick={onCreate ? onCreate : () => createButtonLink && navigate(createButtonLink)} variant={ButtonVariant.primary} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/components/ListEmptyState/ListEmptyState.tsx` around lines 65 - 72, The defaultCreateButton's onClick can call navigate(createButtonLink) when createButtonLink is undefined; update the handler in defaultCreateButton (and related canCreate logic) to guard against this by checking for onCreate first, then if createButtonLink is defined call navigate(createButtonLink), otherwise disable the button or no-op and/or show a console warning; ensure you reference the defaultCreateButton, onCreate, createButtonLink and navigate symbols so the button never invokes navigate(undefined).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/utils/components/ListEmptyState/ListEmptyState.tsx`:
- Around line 65-72: The defaultCreateButton's onClick can call
navigate(createButtonLink) when createButtonLink is undefined; update the
handler in defaultCreateButton (and related canCreate logic) to guard against
this by checking for onCreate first, then if createButtonLink is defined call
navigate(createButtonLink), otherwise disable the button or no-op and/or show a
console warning; ensure you reference the defaultCreateButton, onCreate,
createButtonLink and navigate symbols so the button never invokes
navigate(undefined).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6b9b9c80-b472-48c3-b759-4d16ce351b72
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
src/utils/components/ExternalLink/ExternalLink.tsxsrc/utils/components/ListEmptyState/ListEmptyState.tsxsrc/views/ingresses/list/IngressesList.tsxsrc/views/nads/list/NetworkAttachmentDefinitionList.tsxsrc/views/networkpolicies/list/MultiNetworkPolicyList.tsxsrc/views/networkpolicies/list/NetworkPolicyList.tsxsrc/views/routes/list/RoutesList.tsxsrc/views/services/list/ServiceList.tsx
| createButtonlink={SHARED_DEFAULT_PATH_NEW_RESOURCE_FORM} | ||
| createButtonLink={getResourceURL({ | ||
| activeNamespace: namespace, | ||
| model: { ...NetworkPolicyModel, crd: true }, |
There was a problem hiding this comment.
why you need the crd: true part? and destructing the model?
There was a problem hiding this comment.
getResourceURL checks for crd: true and only builds the group~version~kind portion of the URL if it's true. I need to go through at some point and ensure all the URLs are consistent, but until then I figured this was an easier way to handle the issue.
There was a problem hiding this comment.
well sounds weird the the function is getResourceURL and the crd is true, but it's not in the scope for this PR in my opinion.
|
@pcbailey: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: avivtur, pcbailey The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@pcbailey: Jira Issue OCPBUGS-54248: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-54248 has been moved to the MODIFIED state. DetailsIn 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. |
3 participants
This PR fixes the links used for the Create button in the resource list page empty state component.
Jira: https://redhat.atlassian.net/browse/OCPBUGS-54248
Summary by CodeRabbit
Bug Fixes
Refactor