Fix error stages are not shown for plugin-arch piped created pipeline#5950
Conversation
Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5950 +/- ##
=======================================
Coverage 28.15% 28.15%
=======================================
Files 518 518
Lines 55910 55910
=======================================
Hits 15744 15744
Misses 38914 38914
Partials 1252 1252
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
Warashi
left a comment
Warashi
left a comment
There was a problem hiding this comment.
I left a comment about the optional chaining feature of TypeScript.
| const stages: Stage[][] = []; | ||
| const visibleStages = deployment.stagesList.filter((stage) => stage.visible); | ||
| let visibleStages: Stage[] = []; | ||
| if (deployment.deployTargetsByPluginMap?.length === 0) { |
There was a problem hiding this comment.
I'm not sure about the optional chaining feature in TypeScript, but I think that the deployment.deployTargetsByPluginMap?.length will be undefined when the deployment.deployTargetsByPluginMap is null/undefined. And undefined === 0 will be false.
There was a problem hiding this comment.
Got your point 👀 Should I change this to be as same as this one https://github.com/pipe-cd/pipecd/blob/master/web/src/components/deployments-detail-page/deployment-detail/index.tsx#L80
cc @hongky-1994
There was a problem hiding this comment.
Yep, I think you can change like that.
if (deployment?.deployTargetsByPluginMap?.length) return PIPED_VERSION.V1;
this would reject both "undefined" and "length === 0"
Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
3 participants
What this PR does:
Fix error the stages are not showing for plugin-arch piped created pipeline.
Why we need it:
Previously, the stage visibility was based on pipelineStage.visible field, which is marked as deprecated for pipedv1 (plugin-arch piped). This change updates the UI logic to show the stage as expected without using
pipelineStage.visiblefield. Instead, the UI will usepipelineStage.statusandpipelineStage.rollbackvalues to determine whether to show a stage in the pipeline or not.Which issue(s) this PR fixes:
Part of #4980
Does this PR introduce a user-facing change?: