Skip to content

Conversation

@MikaKerman
Copy link
Contributor

@MikaKerman MikaKerman commented Sep 28, 2025

  • Introduced a new function to append an attribution block to alert messages.
  • Updated the alert message construction to include the attribution block, enhancing message content with a link to Elementary.
  • Refactored imports for better organization and clarity.

Summary by CodeRabbit

  • New Features
    • Alert messages now include a subtle “Powered by Elementary” attribution with a hyperlink to learn more. It appears right after the alert title when available, or at the end of the message otherwise. This is purely visual and does not affect alert delivery or settings. Existing integrations and formatting remain unchanged.

- Introduced a new function  to append an attribution block to alert messages.
- Updated the alert message construction to include the attribution block, enhancing message content with a link to Elementary.
- Refactored imports for better organization and clarity.
@linear
Copy link

linear bot commented Sep 28, 2025

@coderabbitai
Copy link

coderabbitai bot commented Sep 28, 2025

Walkthrough

Adds an internal helper to inject an attribution block ("Powered by Elementary") into alert message bodies and wires it into the alert sending flow. Imports are updated to support new block types. No public APIs change.

Changes

Cohort / File(s) Summary
Alert attribution injection
`elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py`
Added _add_elementary_attribution to build and insert a LinesBlock with a hyperlink attribution; updated imports (LineBlock, LinkBlock, TextBlock, MessageBlock); integrated attribution into _send_alert by transforming the generated alert_message_body before sending.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant A as Alert Generator
  participant H as _add_elementary_attribution
  participant N as Notifier/Sender

  A->>A: Build alert_message_body
  A->>H: Inject attribution
  note right of H: Insert after HeaderBlock if present,<br/>else append at end
  H-->>A: message_body with attribution
  A->>N: Send updated message_body
  N-->>A: Send result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I thump my paws: a link now shines,
A gentle nod in alerting lines—
“Powered by Elementary,” clear and bright,
Tucked after headers, neat and light.
With whisker-twitch and caret’s gleam,
I ship the ping—a tidy stream.
Hop, send, done: a polished dream.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately captures the primary change by indicating that an attribution block is being added to data monitoring alert messages, making it clear and specific to the main objective of the pull request.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch app-111-powered-by-elementary-in-oss-alerts

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

👋 @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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py (3)

66-80: Make attribution insertion idempotent and avoid hardcoding the URL twice.

Add a quick scan to skip insertion when the Elementary link already exists, and factor the URL into a local var to prevent drift.

 def _add_elementary_attribution(body: MessageBody) -> MessageBody:
-    lines = [
+    elementary_url = "https://www.elementary-data.com/"
+
+    # Avoid duplicate attribution if already present
+    if any(
+        isinstance(b, LinesBlock)
+        and any(
+            isinstance(inl, LinkBlock) and inl.url.rstrip("/") == elementary_url.rstrip("/")
+            for line in b.lines
+            for inl in getattr(line, "inlines", [])
+        )
+        for b in body.blocks
+    ):
+        return body
+
+    lines = [
         LineBlock(
             inlines=[
                 TextBlock(text="Powered by"),
                 LinkBlock(
                     text="Elementary",
-                    url="https://www.elementary-data.com/",
+                    url=elementary_url,
                 ),
             ]
         )
     ]
@@
-    first_block_is_header = body.blocks and isinstance(body.blocks[0], HeaderBlock)
+    first_block_is_header = bool(body.blocks) and isinstance(body.blocks[0], HeaderBlock)

Also applies to: 82-95


83-87: Minor typing/readability: avoid relying on list truthiness.

Use an explicit boolean for header detection to keep static type checkers happy and improve clarity.

-first_block_is_header = body.blocks and isinstance(body.blocks[0], HeaderBlock)
+first_block_is_header = bool(body.blocks) and isinstance(body.blocks[0], HeaderBlock)

353-353: Hook point LGTM; consider consistency and config.

Nice placement before send. Optionally, apply attribution to the health-check test message too, or gate via a config flag (e.g., show_attribution: bool) for white-label scenarios.

If you want the test message to include attribution:

 def _send_test_message(self) -> MessageSendResult:
@@
-        test_message = get_health_check_message()
+        test_message = _add_elementary_attribution(get_health_check_message())
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d7f7d9b and 5b9c539.

📒 Files selected for processing (1)
  • elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py (2)
elementary/messages/blocks.py (5)
  • HeaderBlock (119-121)
  • LineBlock (66-69)
  • LinesBlock (137-138)
  • LinkBlock (45-48)
  • TextBlock (39-42)
elementary/messages/message_body.py (1)
  • MessageBody (36-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test / test
  • GitHub Check: code-quality
🔇 Additional comments (1)
elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py (1)

10-17: Imports look good; surface matches usage.

The added block imports and MessageBlock typing align with the helper’s construction and annotations. No issues spotted.

@MikaKerman
Copy link
Contributor Author

image

@MikaKerman MikaKerman merged commit 44bb802 into master Sep 29, 2025
6 checks passed
@MikaKerman MikaKerman deleted the app-111-powered-by-elementary-in-oss-alerts branch September 29, 2025 07:52
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.

3 participants