OCPBUGS-83799: Fix Workloads sidebar ordering when DeploymentConfig capability is disabled#16351
OCPBUGS-83799: Fix Workloads sidebar ordering when DeploymentConfig capability is disabled#16351jhadvig wants to merge 1 commit intoopenshift:mainfrom
Conversation
…apability is disabled The StatefulSets nav item had insertAfter: "deploymentconfigs", which breaks the ordering chain when the DeploymentConfig capability is disabled since the target ID no longer exists. This caused StatefulSets and all subsequent items to be appended to the end of the menu instead of maintaining their expected position after Deployments. Add "deployments" as a fallback target so the chain stays intact regardless of whether DeploymentConfig is enabled. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
openshift-ci-robot
added
jira/severity-moderate
|
@jhadvig: This pull request references Jira Issue OCPBUGS-83799, 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jhadvig 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 |
openshift-ci
Bot
added
component/core
📝 WalkthroughWalkthroughThis change updates navigation extension configuration for the 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/packages/console-app/src/components/nav/__tests__/utils.spec.ts (1)
61-78: Strengthen orphan-case assertions to catch ordering regressions.Current assertions only check presence of
statefulsetsandsecrets; they can pass even if relative ordering regresses. Please assert their relative positions explicitly.Suggested test tightening
const sorted = getIds(getSortedNavExtensions(items)); expect(sorted[0]).toBe('topology'); expect(sorted[1]).toBe('pods'); expect(sorted[2]).toBe('deployments'); - // statefulsets and secrets end up after deployments but order may vary - expect(sorted).toContain('statefulsets'); - expect(sorted).toContain('secrets'); + expect(sorted.indexOf('statefulsets')).toBeGreaterThan(sorted.indexOf('deployments')); + expect(sorted.indexOf('secrets')).toBeGreaterThan(sorted.indexOf('statefulsets'));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-app/src/components/nav/__tests__/utils.spec.ts` around lines 61 - 78, The test currently only asserts presence of 'statefulsets' and 'secrets' allowing ordering regressions; update the spec that uses getSortedNavExtensions/getIds to explicitly assert relative indexes: compute indexes via getIds(sorted).indexOf('deployments'), .indexOf('statefulsets'), and .indexOf('secrets') and add expectations that deploymentsIndex < statefulsetsIndex and deploymentsIndex < secretsIndex, and also assert statefulsetsIndex < secretsIndex (or the intended relative order) to lock in the correct ordering behavior; keep the rest of the test unchanged and reference createNavItem/getSortedNavExtensions/getIds when locating where to add these assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/packages/console-app/src/components/nav/__tests__/utils.spec.ts`:
- Around line 61-78: The test currently only asserts presence of 'statefulsets'
and 'secrets' allowing ordering regressions; update the spec that uses
getSortedNavExtensions/getIds to explicitly assert relative indexes: compute
indexes via getIds(sorted).indexOf('deployments'), .indexOf('statefulsets'), and
.indexOf('secrets') and add expectations that deploymentsIndex <
statefulsetsIndex and deploymentsIndex < secretsIndex, and also assert
statefulsetsIndex < secretsIndex (or the intended relative order) to lock in the
correct ordering behavior; keep the rest of the test unchanged and reference
createNavItem/getSortedNavExtensions/getIds when locating where to add these
assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 3a9eb561-7954-43ff-8e82-91d69a3fadcc
📒 Files selected for processing (2)
frontend/packages/console-app/console-extensions.jsonfrontend/packages/console-app/src/components/nav/__tests__/utils.spec.ts
📜 Review details
🔇 Additional comments (3)
frontend/packages/console-app/console-extensions.json (1)
1156-1156: Good fallback anchor update for StatefulSets ordering.This change correctly preserves the intended Workloads order when
deploymentconfigsis filtered out, while still preferring it when available.frontend/packages/console-app/src/components/nav/__tests__/utils.spec.ts (2)
22-59: Nice coverage for primary and fallback insertAfter behavior.These two tests directly validate the ordering chain with and without
deploymentconfigs, and they align well with the bug scenario.
2-2: Remove this suggestion—LoadedExtensionis not exported from the public API.The SDK intentionally keeps
LoadedExtensionas an internal type (in/src/types). It's not re-exported from the public entrypoint, so this import pattern is the standard and necessary way to access it. This is evident from 49+ usages across the codebase following the same approach. No change needed.> Likely an incorrect or invalid review comment.
|
@jhadvig: 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. |
2 participants
Analysis / Root cause:
The
StatefulSetsnav item inconsole-extensions.jsonusesinsertAfter: "deploymentconfigs". When the DeploymentConfig cluster capability is disabled, thedeploymentconfigsnav item is filtered out entirely (via itsflags.required: ["OPENSHIFT_DEPLOYMENTCONFIG"]), so theinsertAftertarget doesn't exist. The sorting algorithm innav/utils.tsfalls back to appending items with unresolved targets to the end of the list, which breaks the ordering chain for StatefulSets and everything after it (Secrets, ConfigMaps, etc.).Solution description:
Change
insertAfterfrom"deploymentconfigs"to["deploymentconfigs", "deployments"]. TheinsertAfterproperty already supports array fallbacks — the sorting logic takes the first matching ID found. Whendeploymentconfigsis absent, it falls back to inserting afterdeployments, maintaining the correct menu order.Screenshots / screen recording:
Test setup:
Create or use a cluster with the DeploymentConfig capability disabled: docs
Test cases:
getSortedNavExtensionscovering both normal chain and missing-target fallbackBrowser conformance:
Additional info:
Jira: https://redhat.atlassian.net/browse/OCPBUGS-83799
🤖 Generated with Claude Code
Reviewers and assignees:
Summary by CodeRabbit
Chores
Tests