Skip to content

Ele 4262 subscribers in the grouped alerts#1893

Merged
MikaKerman merged 4 commits intomasterfrom
ele-4262-subscribers-in-the-grouped-alerts
Apr 8, 2025
Merged

Ele 4262 subscribers in the grouped alerts#1893
MikaKerman merged 4 commits intomasterfrom
ele-4262-subscribers-in-the-grouped-alerts

Conversation

@MikaKerman
Copy link
Copy Markdown
Contributor

@MikaKerman MikaKerman commented Apr 8, 2025

Summary of Changes

This PR adds support for displaying subscribers in grouped alert messages, improving the visibility of who should be notified for different types of alerts.

Changes

  • Added MessageBuilderConfig to control subscriber display behavior
  • Updated alert message formatting to consistently use plural terms for owners and subscribers
  • Added subscriber information to alert message tests
  • Modified test fixtures to include subscriber scenarios

@linear
Copy link
Copy Markdown

linear Bot commented Apr 8, 2025

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 8, 2025

👋 @MikaKerman
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in this pull request.

@MikaKerman MikaKerman requested a review from ofek1weiss April 8, 2025 09:12
* Introduced MessageBuilderConfig to manage alert group subscribers.
* Enhanced AlertMessageBuilder to conditionally display subscriber information based on configuration.
* Added `subscribers` parameter to various alert message test cases to ensure proper handling of subscriber data.
* Introduced new fixture files for block kit and adaptive card formats reflecting subscriber details.
* Updated existing fixture files to replace placeholder text for subscribers with actual subscriber information.
* Enhanced `MessageBuilderConfig` integration to conditionally display subscriber information in alert messages.
…owners and subscribers

* Modified alert message templates to replace singular terms with plural for owners and subscribers, ensuring clarity in communication.
* Updated various test fixture files to reflect these changes, enhancing consistency across alert messages.
@MikaKerman MikaKerman force-pushed the ele-4262-subscribers-in-the-grouped-alerts branch from 3e5960e to 17622c9 Compare April 8, 2025 09:16

if subscribers := list(set(alert.subscribers)):
if self.config.alert_groups_subscribers:
inlines.append(TextBlock(text="-"))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only add it if there are owners.
also - a pipe (|) might look better here

@MikaKerman MikaKerman had a problem deploying to elementary_test_env April 8, 2025 10:05 — with GitHub Actions Failure
* Updated the AlertMessageBuilder to conditionally format the display of owners and subscribers, ensuring a consistent use of separators.
* Adjusted test cases and fixture files to reflect the new formatting, enhancing the clarity of alert messages.
* Modified the logic for subscriber handling to ensure proper display based on the presence of owners.
@MikaKerman MikaKerman force-pushed the ele-4262-subscribers-in-the-grouped-alerts branch from 858f2f8 to ee7f319 Compare April 8, 2025 10:15
@MikaKerman MikaKerman had a problem deploying to elementary_test_env April 8, 2025 11:43 — with GitHub Actions Failure
@MikaKerman MikaKerman force-pushed the ele-4262-subscribers-in-the-grouped-alerts branch from a285ca0 to ee7f319 Compare April 8, 2025 11:46
@MikaKerman MikaKerman temporarily deployed to elementary_test_env April 8, 2025 11:46 — with GitHub Actions Inactive
@MikaKerman MikaKerman merged commit 4739b80 into master Apr 8, 2025
6 of 7 checks passed
@MikaKerman MikaKerman deleted the ele-4262-subscribers-in-the-grouped-alerts branch April 8, 2025 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants