CNV-55987: Aggregated alerts #348
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@upalatucci: This pull request references CNV-54399 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "4.19.0" version, but no target version was set. In 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. |
upalatucci
force-pushed
the
aggregated-alerts
branch
3 times, most recently
from
47126cd to
6a49da3
Compare
|
@upalatucci: This pull request references CNV-55987 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set. In 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. |
|
/retest |
|
@fkargbo can you please take a look? |
|
Hi @upalatucci I've reviewed the implementation and it looks good! I have some comments listed below:L
I hope this helps. Please let me know if you have any questions. Thanks! |
|
Thank you @fkargbo yes the total amount is not right as I just created fake data for the demo video :D I'll fix everything thanks |
upalatucci
force-pushed
the
aggregated-alerts
branch
from
6a49da3 to
9e2ead1
Compare
|
@upalatucci: This pull request references CNV-55987 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set. In 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. |
|
@fkargbo fixed. Now you can see the video in the pr description |
upalatucci
force-pushed
the
aggregated-alerts
branch
from
9e2ead1 to
de8e017
Compare
Looks good, thanks! |
|
/retest |
web/src/components/alerting/AlertList/AggregateAlertTableRow.tsx
Outdated
Show resolved
Hide resolved
web/src/components/alerting/AlertList/AggregateAlertTableRow.tsx
Outdated
Show resolved
Hide resolved
web/src/components/alerting/AlertList/AggregateAlertTableRow.tsx
Outdated
Show resolved
Hide resolved
| <td className={tableAggregatedAlertClasses[0]} title={title}> | ||
| <MonitoringResourceIcon resource={AlertResource} /> | ||
| <Link | ||
| to={getAlertUrl( |
There was a problem hiding this comment.
I don't think it makes sense to link to the first alert on the outside of expanded area, as each alert in the expanded area has it's own detail page that won't be linked to anymore. In addition, the alert rule for each of the items in the expanded area will be the same.
I think instead what would make sense is to make the link on the outside of the expanded area be the alert rule and then in expanded area change the dropdown for each alert from "View Alerting Rule" to instead just be "View alert".
cc: @fkargbo does that change sound good to you?
There was a problem hiding this comment.
Hi @PeterYurkovich, your suggestion sounds good if each alert has its own details page. I'll research it a bit and run it by @sradco to confirm. Thanks for the feedback! 👍
There was a problem hiding this comment.
We have the virt sync later today, maybe we can touch on it briefly
There was a problem hiding this comment.
I agree with @PeterYurkovich . The link should be to the "Alerting rule details" page.
upalatucci
force-pushed
the
aggregated-alerts
branch
from
de8e017 to
ac23957
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: upalatucci The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@upalatucci: all tests passed! Full PR test history. Your PR dashboard. 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Demo
Screen.Recording.2025-02-14.at.10.29.49.mov
Fix for inner table sticky header
Screen.Recording.2025-02-17.at.12.43.38.mov