Show matchExpressions in Progressive Sync Step Groups (#10177)#256
Open
keithchong wants to merge 1 commit into
Open
Show matchExpressions in Progressive Sync Step Groups (#10177)#256keithchong wants to merge 1 commit into
keithchong wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #256 +/- ##
==========================================
- Coverage 11.92% 11.81% -0.12%
==========================================
Files 154 157 +3
Lines 6272 6373 +101
Branches 2028 2197 +169
==========================================
+ Hits 748 753 +5
+ Misses 5524 5394 -130
- Partials 0 226 +226
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe ApplicationSet graph now exposes progressive sync match expressions through localized context-menu actions and a topology sidebar that renders the selected step as YAML. Graph nodes also carry API metadata, while related translations, YAML tooling, and label styling are updated. ChangesApplicationSet match expressions
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant StepGroupComponent
participant AppSetContextMenuItem
participant ApplicationSetGraphView
participant AppSetMatchExpressionsTab
User->>StepGroupComponent: Open step-group context menu
StepGroupComponent->>AppSetContextMenuItem: Show match-expressions action
User->>AppSetContextMenuItem: Select action
AppSetContextMenuItem->>ApplicationSetGraphView: Set showMatchExpressions to true
ApplicationSetGraphView->>AppSetMatchExpressionsTab: Render selected rolling-sync step
AppSetMatchExpressionsTab-->>User: Display step YAML
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/gitops/components/appset/graph/ApplicationSetGraphView.tsx (1)
371-371: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPrefer
.getType()over ID-suffix matching for node-type detection.
selectedNode?.getId().endsWith('-step-group')is brittle compared to the.getType() === 'step-group'check used consistently elsewhere in this file/module for identifying step-group nodes.🤖 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/gitops/components/appset/graph/ApplicationSetGraphView.tsx` at line 371, Update the isStepGroup calculation to use selectedNode?.getType() === 'step-group' instead of matching the node ID suffix, consistent with the other step-group detection logic in the module.
🤖 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.
Inline comments:
In `@package.json`:
- Line 81: Move the yaml package entry from devDependencies to dependencies in
package.json, preserving its current version constraint so the runtime import in
AppSetMatchExpressionsTab.tsx remains available in production installs.
In `@src/gitops/components/appset/AppSetMatchExpressionsTab.tsx`:
- Around line 13-23: Update AppSetMatchExpressionsTabProps so customData uses
the established shape from ApplicationSetGraphView.tsx, including its step
field, instead of object. Before parseInt and any translation-string
interpolation in AppSetMatchExpressionsTab, coerce customData.step to a string
and then parse it, preserving the existing step-index lookup behavior.
In `@src/gitops/components/appset/graph/ApplicationSetGraphView.tsx`:
- Around line 351-448: Update getTopologySideBar to remove the React.useMemo
call and hoist the stable pages definition to module scope. Refactor the
selectedNode rendering logic into a small helper or equivalent conditional
structure so no nested ternary remains, and remove the redundant selectedNode
!== undefined check while preserving the existing sidebar output for no
selection, regular nodes, and step groups.
In `@src/gitops/components/appset/graph/graph-utils.tsx`:
- Around line 235-247: Update the key, operator, and values access in the graph
step logic to use optional chaining when indexing rollingSync.steps and
matchExpressions.values, including `?.[]` for array access. Extract the selected
step or match expression if useful to avoid repeating the long chain, while
preserving empty-string fallbacks for missing data.
In `@src/gitops/components/appset/graph/nodes/StepGroupComponent.tsx`:
- Line 322: Replace the concatenated step translation in
src/gitops/components/appset/graph/nodes/StepGroupComponent.tsx#L322-L322 with
the interpolation pattern using the step value, and add the “Step {{x}}”
translation key to every locale file. Apply the same change in
src/gitops/components/appset/AppSetMatchExpressionsTab.tsx#L37-L37 using
“Progressive Sync Step {{x}}” with customData?.step, and add that matching key
to every locale file.
---
Nitpick comments:
In `@src/gitops/components/appset/graph/ApplicationSetGraphView.tsx`:
- Line 371: Update the isStepGroup calculation to use selectedNode?.getType()
=== 'step-group' instead of matching the node ID suffix, consistent with the
other step-group detection logic in the module.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: b69cf727-962e-42d6-8eef-69c63d2afe78
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
locales/en/plugin__gitops-plugin.jsonlocales/ja/plugin__gitops-plugin.jsonlocales/ko/plugin__gitops-plugin.jsonlocales/zh/plugin__gitops-plugin.jsonpackage.jsonsrc/gitops/components/application/graph/ApplicationGraphView.scsssrc/gitops/components/application/graph/graph-utils.tsxsrc/gitops/components/appset/AppSetMatchExpressionsTab.tsxsrc/gitops/components/appset/graph/ApplicationSetGraphView.tsxsrc/gitops/components/appset/graph/graph-utils.tsxsrc/gitops/components/appset/graph/nodes/StepGroupComponent.tsx
keithchong
changed the title
Signed-off-by: Keith Chong <kykchong@redhat.com>
keithchong
force-pushed
the
10177-MatchExpressionsInStepGroupTitle
branch
from
July 15, 2026 18:01
7b31b33 to
fbd0b43
Compare
keithchong
changed the title
Collaborator
Author
|
Just let me know! ✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/gitops/components/appset/graph/graph-utils.tsx (1)
235-241: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImprove label formatting and provide a fallback for empty expressions.
Concatenating the parts with hardcoded spaces (
key + ' ' + operator + ' ' + values) will result in trailing spaces for operators that do not have values (e.g.,Exists), or an almost empty label (" ") ifmatchExpressionsis undefined.Consider filtering out empty values and providing a fallback label so the step group always has a meaningful name in the graph.
♻️ Proposed refactor
const stepGroupApps = stepGroupAppsMap.get(step); const stepMatchExpression = applicationSet.spec?.strategy?.rollingSync?.steps?.[ parseInt(step) - 1 ]?.matchExpressions?.at(0); const key = stepMatchExpression?.key || ''; const operator = stepMatchExpression?.operator || ''; const values = stepMatchExpression?.values?.[0] || ''; + const labelText = [key, operator, values].filter(Boolean).join(' ') || `Progressive Sync Step ${step}`; if (stepGroupApps.length === 0 && hasApplicationStatus) { stepGroupAppsMap.set(step.toString(), [step + '-filler-node']); ... const stepGroup = { anchor: CenterAnchor, id: step + '-step-group', type: 'step-group', group: true, rank: 3, - label: key + ' ' + operator + ' ' + values, + label: labelText, children: [...stepGroupApps],Also applies to: 285-285
🤖 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/gitops/components/appset/graph/graph-utils.tsx` around lines 235 - 241, Update the step-group label construction near stepMatchExpression to build the label from non-empty key, operator, and values parts, avoiding extra spaces for operators without values. When matchExpressions is missing or all parts are empty, use a meaningful fallback label so every step group remains named in the graph.
🤖 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.
Nitpick comments:
In `@src/gitops/components/appset/graph/graph-utils.tsx`:
- Around line 235-241: Update the step-group label construction near
stepMatchExpression to build the label from non-empty key, operator, and values
parts, avoiding extra spaces for operators without values. When matchExpressions
is missing or all parts are empty, use a meaningful fallback label so every step
group remains named in the graph.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: abb86971-bb4b-4c7e-bc9e-179d9b37ef01
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
locales/en/plugin__gitops-plugin.jsonlocales/ja/plugin__gitops-plugin.jsonlocales/ko/plugin__gitops-plugin.jsonlocales/zh/plugin__gitops-plugin.jsonpackage.jsonsrc/gitops/components/application/graph/ApplicationGraphView.scsssrc/gitops/components/application/graph/graph-utils.tsxsrc/gitops/components/appset/AppSetMatchExpressionsTab.tsxsrc/gitops/components/appset/graph/ApplicationSetGraphView.tsxsrc/gitops/components/appset/graph/graph-utils.tsxsrc/gitops/components/appset/graph/nodes/StepGroupComponent.tsx
🚧 Files skipped from review as they are similar to previous changes (9)
- src/gitops/components/application/graph/graph-utils.tsx
- src/gitops/components/appset/AppSetMatchExpressionsTab.tsx
- src/gitops/components/appset/graph/nodes/StepGroupComponent.tsx
- locales/ko/plugin__gitops-plugin.json
- src/gitops/components/application/graph/ApplicationGraphView.scss
- locales/zh/plugin__gitops-plugin.json
- locales/ja/plugin__gitops-plugin.json
- locales/en/plugin__gitops-plugin.json
- src/gitops/components/appset/graph/ApplicationSetGraphView.tsx
aali309
reviewed
Jul 17, 2026
aali309
left a comment
aali309
left a comment
Collaborator
There was a problem hiding this comment.
Lets merge the pf6 PR first?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See GITOPS-10177