CNV-75852: fix duplicate favorite star in Catalog InstanceTypes page #3323
Conversation
- also fixes a whole section reload on favoriting a bootable volume
|
@adamviktora: This pull request references CNV-75852 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 bug to target the "4.22.0" version, but no target version was set. 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. |
📝 WalkthroughWalkthroughPatternFly package dependencies pinned from caret ranges to exact versions in package.json. The Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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 |
|
@adamviktora: This pull request references CNV-75852 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 bug to target the "4.22.0" version, but no target version was set. 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. |
|
@adamviktora: This pull request references CNV-75852 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 bug to target the "4.22.0" version, but no target version was set. 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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/views/catalog/CreateFromInstanceTypes/CreateFromInstanceType.tsx (1)
96-96: Consider removing the unnecessary type assertion.The type assertion
as []onfavoritesis unnecessary and potentially misleading sincefavoritesis already typed through its default value assignment on line 57. The prop should accept the inferred type without coercion.As per coding guidelines, avoid using type assertions unless absolutely necessary for type narrowing.
🔎 Suggested improvement
- favorites={[favorites as [], updaterFavorites]} + favorites={[favorites, updaterFavorites]}package.json (1)
8-18: Consider establishing a process for regular PatternFly version reviews.Pinning to exact versions prevents automatic security and bug fixes. Since the PR description mentions this is a "short-term fix," consider:
- Establishing a regular cadence (e.g., monthly) to review and update PatternFly versions
- Setting up automated dependency checks (e.g., Dependabot, Renovate) with manual approval
- Documenting the version alignment requirement with OpenShift Console in the project README
- Tracking this as technical debt for a longer-term solution
This will help balance version stability with security maintenance.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.jsonsrc/views/catalog/CreateFromInstanceTypes/CreateFromInstanceType.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.tsx
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.tsx: Use.tsxfile extension for all React components. One component per file with no nested components.
Use PascalCase for all React component names (e.g.,HeaderTop.tsx)
Use functional components by default. Only use class components for specific lifecycle methods unavailable in functional components (e.g.,componentDidCatch)
Use default exports for all React components
UseReact.lazyandSuspensefor lazy loading components
Files:
src/views/catalog/CreateFromInstanceTypes/CreateFromInstanceType.tsx
**/*.{tsx,scss}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Use project-based class names on components as anchors for styling rules rather than relying on PatternFly class names
Files:
src/views/catalog/CreateFromInstanceTypes/CreateFromInstanceType.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{tsx,ts}: Extract logic from components into custom hooks or utility files to improve testability and component maintainability
Use React memoization tools (React.memo,useMemo,useCallback) to avoid unnecessary re-renders
Always specify dependencies inuseEffectto avoid unnecessary re-renders or missed updates. Use an empty array[]if no dependencies are required
Files:
src/views/catalog/CreateFromInstanceTypes/CreateFromInstanceType.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{ts,tsx,js,jsx}: Keep files under 150 lines whenever possible
Use descriptive names for variables, functions, and components. Avoid abbreviations unless widely recognized
Keep functions short and focused on one action. Apply Red → Green → Refactor methodology
Avoid hardcoded values (magic numbers) and define them as constants for easy adjustments and readability
Files:
src/views/catalog/CreateFromInstanceTypes/CreateFromInstanceType.tsx
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Define constants in utility files with uppercase and underscore-separated naming (e.g.,
API_URL)
Files:
src/views/catalog/CreateFromInstanceTypes/CreateFromInstanceType.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{ts,tsx}: Prefer usingtypeinstead ofinterfacefor defining the shapes of objects or functions in TypeScript
If a type is exported, add it to a utility file
Avoid usinganytype in TypeScript. Useunknowninstead and narrow the type as needed
Always explicitly define return types for functions rather than relying on TypeScript type inference
Files:
src/views/catalog/CreateFromInstanceTypes/CreateFromInstanceType.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-09T17:30:56.131Z
Learnt from: rszwajko
Repo: kubevirt-ui/kubevirt-plugin PR: 3235
File: src/views/checkups/self-validation/details/tabs/details/components/CheckupsSelfValidationDetailsDescriptionList.tsx:26-93
Timestamp: 2025-12-09T17:30:56.131Z
Learning: When using the Timestamp component from openshift-console/dynamic-plugin-sdk, you can pass sentinel values like NO_DATA_DASH directly to the timestamp prop without wrapping in conditional rendering. The component handles invalid/missing values gracefully. This applies to all TSX files that render the Timestamp component; ensure you do not add extra conditional logic for such values and rely on the component's internal handling. Reference: OpenShift Console Timestamp.tsx implementation.
Applied to files:
src/views/catalog/CreateFromInstanceTypes/CreateFromInstanceType.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
src/views/catalog/CreateFromInstanceTypes/CreateFromInstanceType.tsx (2)
57-60: LGTM - RemovingloadedFavoritesaddresses the section reload issue.The default value
favorites = []ensures the component handles the initial state gracefully. This change allows the component to render without waiting for favorites to load, which addresses the full section reload issue mentioned in the PR description.
62-68: LGTM - Conditional logic correctly simplified.Removing
loadedFavoritesfrom the loading check is consistent with the hook destructuring change. The component will now proceed with the default empty favorites array, allowing BootableVolumeList to render without waiting for favorites to load, which resolves the duplicate star issue.package.json (1)
8-18: All PatternFly versions are available and compatible.All 11 pinned PatternFly packages exist on npm with the specified versions. The
@patternfly/[email protected]is correctly aligned with the PatternFly 6.2.x series—despite its higher major version number, react-charts 8.2.x is intentionally released together with PatternFly 6.x.The pinned versions should resolve the styling issues mentioned in the PR description.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adamviktora, batyana 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 |
3 participants
📝 Description
The issue was that we updated our
package-lock.json(#3316), including the Patternfly dependencies that are with caret (^6.2.2) - so it updated Patternfly to 6.4.0. Console also has^6.2.2but their lockfile was last generated with Patternfly 6.2.2 as the newest version. Loading these PF versions together with Console breaks some styling.Short term fix is to have specific Patternfly versions in package.json to align with Console.
🎥 Demo
Before:
Screen.Recording.2026-01-06.at.9.09.12.mov
After:
Screen.Recording.2026-01-05.at.16.14.41.mov
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.