-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(Single recommendation page): add Impacted column #340
feat(Single recommendation page): add Impacted column #340
Conversation
@RedHatInsights/team-interact, please take a look 🌞 |
0f789d1
to
1b59344
Compare
Ran it locally, tested the functionality |
}, | ||
{ | ||
"cluster": "f7331e9a-2f59-484d-af52-338d56165df5", | ||
"cluster_name": "custom cluster name 2", | ||
"last_checked_at": "2020-11-07T08:32:37.690Z", | ||
"meta": { | ||
"cluster_version": "4.18.12" | ||
} | ||
}, | ||
"impacted": "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the server will not respond with empty string but with the field missing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are true, they marked it as optional, then this key can be possibly omitted. I'll rewrite the code so that it also considers missing "impacted"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ikerreyes, updated, PTAL
src/Components/AffectedClustersTable/AffectedClustersTable.spec.ct.js
Outdated
Show resolved
Hide resolved
…e-rec-impacted-col
Codecov Report
@@ Coverage Diff @@
## master #340 +/- ##
==========================================
+ Coverage 79.75% 80.47% +0.72%
==========================================
Files 25 25
Lines 1121 1127 +6
Branches 425 431 +6
==========================================
+ Hits 894 907 +13
+ Misses 227 220 -7
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good. Just a minor improvement we can discuss.
src/Components/AffectedClustersTable/AffectedClustersTable.spec.ct.js
Outdated
Show resolved
Hide resolved
# [1.5.0](v1.4.10...v1.5.0) (2022-07-08) ### Features * **Single recommendation page:** add Impacted column ([#340](#340)) ([be12a3e](be12a3e))
🎉 This PR is included in version 1.5.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Implements https://issues.redhat.com/browse/CCXDEV-7462.
The "Impacted" column displays the date when the recommendation first hit the particular cluster. The column is sortable, no filter is required.