Show matchExpressions in Progressive Sync Step Groups (#10177) - #256
Merged
aali309 merged 2 commits intoAug 7, 2026
Merged
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #256 +/- ##
==========================================
- Coverage 11.92% 11.30% -0.63%
==========================================
Files 154 169 +15
Lines 6272 6679 +407
Branches 2028 2235 +207
==========================================
+ Hits 748 755 +7
- Misses 5524 5923 +399
- Partials 0 1 +1
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 a localized context-menu action and topology sidebar. Graph nodes include API metadata. The change also adds YAML support, updates translations, applies PatternFly v6 classes, and adjusts graph styling. ChangesApplicationSet match expressions and UI updates
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 formatted 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
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?
Signed-off-by: Keith Chong <kykchong@redhat.com>
keithchong
force-pushed
the
10177-MatchExpressionsInStepGroupTitle
branch
from
August 5, 2026 20:54
fbd0b43 to
3bd1dab
Compare
aali309
reviewed
Aug 6, 2026
aali309
left a comment
aali309
left a comment
Collaborator
There was a problem hiding this comment.
Hey Keith, Looks good just a few comments/questions, then we are good to go! Also should we make this opaque?
Collaborator
Author
The opaque issue is because, at the time when this PR was first created, we didn't merge the PF5->PF6 change and it was working fine. After the PF6 change, this issue got exposed. So yep, need to update it. |
Signed-off-by: Keith Chong <kykchong@redhat.com>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@locales/ko/plugin__gitops-plugin.json`:
- Around line 104-111: Translate the newly added English locale values using
reviewed translations: update the ApplicationSet entries at
locales/ko/plugin__gitops-plugin.json lines 104-111 and
locales/zh/plugin__gitops-plugin.json lines 104-111, and update the rollout
entries at locales/ko/plugin__gitops-plugin.json lines 264-265 and
locales/zh/plugin__gitops-plugin.json lines 264-265. Preserve the existing keys
and valid JSON structure.
🪄 Autofix
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: Pro Plus
Run ID: 2700fd41-9413-4868-bb85-42dba9207590
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
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/ApplicationDetailsTab.tsxsrc/gitops/components/application/ApplicationSyncStatusTab.tsxsrc/gitops/components/application/graph/ApplicationGraphView.scsssrc/gitops/components/application/graph/graph-utils.tsxsrc/gitops/components/appset/AppSetDetailsTab.scsssrc/gitops/components/appset/AppSetDetailsTab.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.tsxsrc/gitops/components/imageupdater/ImageUpdaterDetailsTab.tsxsrc/gitops/components/project/ProjectDetailsTab.tsxsrc/gitops/components/rollout/RolloutDetailsTab.tsxsrc/gitops/components/rollout/components/AnalysisRunStatus/AnalysisRunStatus.scsssrc/gitops/components/shared/BaseDetailsSummary/BaseDetailsSummary.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- package.json
- src/gitops/components/application/graph/ApplicationGraphView.scss
- src/gitops/components/application/graph/graph-utils.tsx
- locales/ja/plugin__gitops-plugin.json
- src/gitops/components/appset/graph/nodes/StepGroupComponent.tsx
- src/gitops/components/appset/graph/graph-utils.tsx
- src/gitops/components/appset/AppSetMatchExpressionsTab.tsx
- src/gitops/components/appset/graph/ApplicationSetGraphView.tsx
Comment on lines
+104
to
+111
| "Progressive Sync Step {{x}}": "Progressive Sync Step {{x}}", | ||
| "Applications": "Applications", | ||
| "Show all match expressions": "Show all match expressions", | ||
| "Edit ApplicationSet": "Edit ApplicationSet", | ||
| "Delete ApplicationSet": "Delete ApplicationSet", | ||
| "View Graph": "View Graph", | ||
| "Match Expressions": "Match Expressions", | ||
| "Name must be unique within a namespace.": "Name must be unique within a namespace.", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Translate the new entries in the Korean and Chinese locale files.
The new values are English source text in both non-English locale files. This makes the new ApplicationSet and rollout UI display English instead of localized text.
locales/ko/plugin__gitops-plugin.json#L104-L111: Replace the ApplicationSet values with reviewed Korean translations.locales/ko/plugin__gitops-plugin.json#L264-L265: Replace the rollout values with reviewed Korean translations.locales/zh/plugin__gitops-plugin.json#L104-L111: Replace the ApplicationSet values with reviewed Chinese translations.locales/zh/plugin__gitops-plugin.json#L264-L265: Replace the rollout values with reviewed Chinese translations.
📍 Affects 2 files
locales/ko/plugin__gitops-plugin.json#L104-L111(this comment)locales/ko/plugin__gitops-plugin.json#L264-L265locales/zh/plugin__gitops-plugin.json#L104-L111locales/zh/plugin__gitops-plugin.json#L264-L265
🤖 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 `@locales/ko/plugin__gitops-plugin.json` around lines 104 - 111, Translate the
newly added English locale values using reviewed translations: update the
ApplicationSet entries at locales/ko/plugin__gitops-plugin.json lines 104-111
and locales/zh/plugin__gitops-plugin.json lines 104-111, and update the rollout
entries at locales/ko/plugin__gitops-plugin.json lines 264-265 and
locales/zh/plugin__gitops-plugin.json lines 264-265. Preserve the existing keys
and valid JSON structure.
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