Add a footer to email notifications

Fixes: #895
Signed-off-by: Aurélien Bompard <[email protected]>

changelog.d/895.added

@@ -0,0 +1 @@
+Add a footer to email notifications with a link to the rule that generated it

fmn/consumer/consumer.py

@@ -87,6 +87,7 @@ async def _handle(self, message: message.Message, db: AsyncSession):
                 # Record that the rule generated a notification
                 db.add(Generated(rule=rule, count=1))
         # Send the deduplicated notifications
+        print(notifications)
         for notification in notifications:
             await self._send(notification, message)
 

fmn/core/config.py

@@ -60,6 +60,7 @@ class ServicesModel(BaseModel):
 
 
 class Settings(BaseSettings):
+    public_url: str = "https://notifications.fedoraproject.org"
     cors_origins: str = "https://notifications.fedoraproject.org"
     oidc_provider_url: str = "https://id.fedoraproject.org/openidc"
     oidc_conf_endpoint: str = "/.well-known/openid-configuration"

fmn/database/model/destination.py

@@ -6,9 +6,11 @@
 from typing import TYPE_CHECKING
 
 from httpx import AsyncClient, HTTPStatusError
-from sqlalchemy import Column, ForeignKey, Integer, String, UnicodeText
+from sqlalchemy import Column, ForeignKey, Integer, String, UnicodeText, select
+from sqlalchemy.ext.asyncio import async_object_session
 from sqlalchemy.orm import relationship
 
+from ...core.config import get_settings
 from ..main import Base
 from .generation_rule import GenerationRule
 
@@ -37,17 +39,26 @@ class Destination(Base):
     async def generate(self, message: "Message") -> "Notification.content":
         app_name = f"[{message.app_name}] " if message.app_name else ""
         url = message.url if message.url else ""
+        settings = get_settings()
         if self.protocol == "email":
             body = f"{message!s}\n{url}"
             extra = await get_extra(message)
             if extra:
                 body = f"{body}\n{extra}"
+
+            # Find the URL of the Rule that generated this notification
+            session = async_object_session(self)
+            result = await session.execute(
+                select(GenerationRule).where(GenerationRule.id == self.generation_rule_id)
+            )
+            rule_id = result.scalar_one().rule_id
             return {
                 "headers": {
                     "To": self.address,
                     "Subject": f"{app_name}{message.summary}",
                 },
                 "body": body,
+                "footer": f"Sent by Fedora Notifications: {settings.public_url}/rules/{rule_id}",
             }
         elif self.protocol == "irc":
             return {"to": self.address, "message": f"{app_name}{message.summary} {url}"}

fmn/rules/notification.py

@@ -19,6 +19,19 @@ class EmailNotificationHeaders(FrozenModel):
 class EmailNotificationContent(FrozenModel):
     headers: EmailNotificationHeaders
     body: str
+    footer: str | None = None
+
+    def __eq__(self, other):
+        if isinstance(other, self.__class__):
+            return self.model_dump(exclude=["footer"]) == other.model_dump(exclude=["footer"])
+        return super().__eq__(other)
+
+    def __hash__(self):
+        # Don't include the footer in the hash to be able to use set() to de-duplicate
+        # notifications coming from different rules.
+        internal_dict = self.__dict__.copy()
+        del internal_dict["footer"]
+        return hash(self.__class__) + hash(tuple(internal_dict.values()))
 
 
 class EmailNotification(FrozenModel):

fmn/sender/email.py

@@ -28,7 +28,10 @@ async def handle(self, message):
         notif["From"] = self._config["from"]
         for name, value in message["headers"].items():
             notif[name] = value
-        notif.set_content(message["body"])
+        body = message["body"]
+        if message.get("footer") is not None:
+            body = f"{body}\n\n-- \n{message['footer']}"
+        notif.set_content(body)
         log.info("Sending email to %s with subject %s", notif["To"], notif["Subject"])
         try:
             await self._smtp.send_message(notif)

tests/consumer/test_consumer.py

@@ -147,8 +147,9 @@ async def test_consumer_call_tracked(
     n = c.send_queue.send.call_args[0][0]
     assert n.protocol == "email"
     assert n.content == EmailNotificationContent(
-        body="Body of message on dummy.topic\n",
+        body=("Body of message on dummy.topic\n"),
         headers={"Subject": "Message on dummy.topic", "To": "[email protected]"},
+        footer="Sent by Fedora Notifications: https://notifications.fedoraproject.org/rules/1",
     )
 
     result = await db_async_session.execute(select(model.Generated))

tests/consumer/test_send_queue.py

@@ -49,7 +49,10 @@ async def test_send_queue_send(connection, notif):
     sq._exchange.publish.assert_called_once()
     assert sq._exchange.publish.call_args.kwargs.get("routing_key") == "send.email"
     sent_msg = sq._exchange.publish.call_args.args[0]
-    assert sent_msg.body == b'{"headers": {"To": "dummy", "Subject": "dummy"}, "body": "dummy"}'
+    assert (
+        sent_msg.body == b'{"headers": {"To": "dummy", "Subject": "dummy"}, '
+        b'"body": "dummy", "footer": null}'
+    )
 
 
 async def test_send_queue_close(connection):

tests/database/model/test_destination.py

@@ -7,11 +7,31 @@
 import httpx
 import pytest
 
+from fmn.database import model
 from fmn.database.model.destination import Destination, get_extra
 
 
-async def test_email(make_mocked_message):
-    d = Destination(id=1, protocol="email", address="[email protected]")
+@pytest.fixture
+async def rule_obj(db_async_session):
+    user = model.User(name="allkneelbeforezod")
+    tracking_rule = model.TrackingRule(name="datrackingrule")
+    generation_rules = [model.GenerationRule()]
+    rule = model.Rule(user=user, tracking_rule=tracking_rule, generation_rules=generation_rules)
+    db_async_session.add(rule)
+    await db_async_session.flush()
+    yield rule
+    await db_async_session.rollback()
+
+
+async def test_email(make_mocked_message, db_async_session, rule_obj):
+    d = Destination(
+        id=1,
+        protocol="email",
+        address="[email protected]",
+        generation_rule_id=rule_obj.generation_rules[0].id,
+    )
+    db_async_session.add(d)
+
     message = make_mocked_message(
         topic="dummy",
         body={
@@ -25,14 +45,21 @@ async def test_email(make_mocked_message):
     assert result == {
         "headers": {"To": "[email protected]", "Subject": "[dummy] dummy summary"},
         "body": "dummy content\nhttps://dummy.org/dummylink",
+        "footer": "Sent by Fedora Notifications: https://notifications.fedoraproject.org/rules/1",
     }
 
 
-async def test_email_with_extra(make_mocked_message, mocker):
+async def test_email_with_extra(make_mocked_message, mocker, db_async_session, rule_obj):
     mocker.patch(
         "fmn.database.model.destination.get_extra", mock.AsyncMock(return_value="DUMMY EXTRA")
     )
-    d = Destination(id=1, protocol="email", address="[email protected]")
+    d = Destination(
+        id=1,
+        protocol="email",
+        address="[email protected]",
+        generation_rule_id=rule_obj.generation_rules[0].id,
+    )
+    db_async_session.add(d)
     message = make_mocked_message(
         topic="dummy",
         body={
@@ -42,10 +69,13 @@ async def test_email_with_extra(make_mocked_message, mocker):
             "url": "https://dummy.org/dummylink",
         },
     )
+
     result = await d.generate(message)
+
     assert result == {
         "headers": {"To": "[email protected]", "Subject": "[dummy] dummy summary"},
         "body": "dummy content\nhttps://dummy.org/dummylink\nDUMMY EXTRA",
+        "footer": "Sent by Fedora Notifications: https://notifications.fedoraproject.org/rules/1",
     }
 
 

tests/database/model/test_rule.py

@@ -31,18 +31,26 @@ async def test_select_related(self, db_async_session, db_obj):
         assert len(rule.generation_rules) == 1
         assert all(isinstance(gr, model.GenerationRule) for gr in rule.generation_rules)
 
-    async def test_handle_match(db_async_session, db_obj, mocker, make_mocked_message):
+    async def test_handle_match(self, db_async_session, db_obj, mocker, make_mocked_message):
         message = make_mocked_message(topic="dummy", body={"foo": "bar"})
-        tr = model.TrackingRule(name="dummy")
+        tr = db_obj.tracking_rule
+        # tr = model.TrackingRule(name="dummy", rule_id=db_obj.id)
         tr_matches = mocker.patch.object(tr, "matches", return_value=True)
-        gr = model.GenerationRule()
-        db_obj.user = model.User(name="dummy")
-        db_obj.tracking_rule = tr
-        db_obj.generation_rules = [gr]
+        # gr = model.GenerationRule()
+        # db_obj.user = model.User(name="dummy")
+        # db_obj.tracking_rule = tr
+        # db_obj.generation_rules = [gr]
+        gr = db_obj.generation_rules[0]
         for i in range(1, 4):
-            gr.destinations.append(model.Destination(protocol="email", address=f"n{i}"))
+            db_async_session.add(
+                model.Destination(protocol="email", address=f"n{i}", generation_rule=gr)
+            )
+        await db_async_session.flush()
         requester = Mock()
-        result = [n async for n in db_obj.handle(message, requester)]
+        rule_result = await db_async_session.execute(
+            db_obj.select_related().filter_by(id=db_obj.id)
+        )
+        result = [n async for n in rule_result.scalar_one().handle(message, requester)]
         tr_matches.assert_called_once_with(message, requester)
         assert len(result) == 3
         assert [n.content.headers.model_dump()["To"] for n in result] == ["n1", "n2", "n3"]

tests/rules/test_notification.py

@@ -0,0 +1,11 @@
+# SPDX-FileCopyrightText: Contributors to the Fedora Project
+#
+# SPDX-License-Identifier: MIT
+
+from fmn.rules.notification import EmailNotificationContent, IRCNotificationContent
+
+
+def test_compare_other():
+    content = EmailNotificationContent(headers={"To": "dummy", "Subject": "dummy"}, body="dummy")
+    other = IRCNotificationContent(to="dummy", message="dummy")
+    assert content != other

tests/sender/test_email.py

@@ -54,3 +54,25 @@ async def test_email_disconnected():
 
     assert smtp.send_message.call_count == 2
     smtp.connect.assert_called_once_with()
+
+
+async def test_email_handle_with_footer():
+    smtp = MagicMock(spec=SMTP)
+    handler = EmailHandler({"from": "FMN <[email protected]>"})
+    handler._smtp = smtp
+
+    await handler.handle(
+        {
+            "headers": {"To": "[email protected]", "Subject": "Testing"},
+            "body": "This is a test",
+            "footer": "This is a footer.",
+        }
+    )
+
+    smtp.send_message.assert_called_once()
+
+    sent = smtp.send_message.call_args[0][0]
+    assert sent["To"] == "[email protected]"
+    assert sent["Subject"] == "Testing"
+    assert sent.get_body().get_content() == "This is a test\n\n-- \nThis is a footer.\n"
+    assert sent.get_content_type() == "text/plain"