Fix #1260: Add a functional test and fix code

RemoteSettings.Dockerfile

@@ -99,4 +99,4 @@ ENV GRANIAN_STATIC_PATH_ROUTE=/attachments
 ENV GRANIAN_STATIC_PATH_MOUNT=/tmp/attachments
 
 # create directories for volume mounts used in browser tests / local development
-RUN mkdir -p -m 777 /app/mail && mkdir -p -m 777 /tmp/attachments
+RUN mkdir -p -m 777 /app/mail && mkdir -p -m 777 /app/slack && mkdir -p -m 777 /tmp/attachments

browser-tests/conftest.py

@@ -29,6 +29,7 @@ def fetch_changeset(self, **kwargs) -> list[dict]:
 DEFAULT_EDITOR_AUTH = os.getenv("EDITOR_AUTH", "editor:pass")
 DEFAULT_REVIEWER_AUTH = os.getenv("REVIEWER_AUTH", "reviewer:pass")
 DEFAULT_MAIL_DIR = os.getenv("MAIL_DIR", "mail")
+DEFAULT_SLACK_DIR = os.getenv("SLACK_DIR", "")
 
 
 Auth = Tuple[str, str]
@@ -68,6 +69,13 @@ def pytest_addoption(parser):
         "string to disable email tests. Should be disabled for browser/integration "
         "tests",
     )
+    parser.addoption(
+        "--slack-dir",
+        action="store",
+        default=DEFAULT_SLACK_DIR,
+        help="Directory where kinto-slack writes notification files (slack.debug_dir). "
+        "Set as empty string to disable Slack tests.",
+    )
 
 
 @pytest.fixture(scope="session")
@@ -127,6 +135,15 @@ def mail_dir(request) -> str:
     return directory
 
 
+@pytest.fixture(scope="session")
+def slack_dir(request) -> str:
+    directory = request.config.getoption("--slack-dir")
+    if not directory:
+        pytest.skip("SLACK_DIR not set. Skipping Slack test.")
+    os.makedirs(directory, exist_ok=True)
+    return directory
+
+
 @pytest.fixture(scope="session")
 def request_session(server) -> requests.Session:
     session = requests.Session()

browser-tests/plugins/test_slack.py

@@ -0,0 +1,54 @@
+import json
+import os
+import re
+
+
+def test_slack_plugin(
+    setup_client,
+    editor_client,
+    slack_dir: str,
+):
+    existing_files = set(os.listdir(slack_dir))
+
+    if setup_client:
+        setup_client.patch_bucket(
+            data={
+                "kinto-slack": {
+                    "hooks": [
+                        {
+                            "event": "kinto_remote_settings.signer.events.ReviewRequested",
+                            "channel": "#reviews",
+                            "template": "{user_id} requested review for {changes_count} changes ({comment}) on {bucket_id}/{collection_id}.",
+                        }
+                    ]
+                }
+            },
+        )
+
+    bucket_metadata = editor_client.get_bucket()
+    slack_hooks = bucket_metadata["data"]["kinto-slack"]["hooks"]
+    assert [h for h in slack_hooks if "ReviewRequested" in h["event"]], (
+        "Slack hook not found"
+    )
+
+    # Create record, will set status to "work-in-progress"
+    editor_client.create_record(data={"hola": "mundo"})
+    # Request review!
+    editor_client.patch_collection(
+        data={"status": "to-review", "last_editor_comment": "looks good"}
+    )
+
+    files_created = set(os.listdir(slack_dir)) - existing_files
+    assert files_created, "No Slack notifications sent"
+    assert len(files_created) == 1, "Too many Slack notifications sent"
+
+    notification_file = files_created.pop()
+    assert notification_file.endswith(".json")
+
+    with open(os.path.join(slack_dir, notification_file)) as f:
+        payload = json.load(f)
+
+    assert payload["channel"] == "#reviews"
+    assert "integration-tests" in payload["text"]
+    assert "looks good" in payload["text"]
+    assert re.search(r"\d+ changes", payload["text"])

docker-compose.yml

@@ -1,6 +1,7 @@
 volumes:
   db-data:
   debug-mail:
+  debug-slack:
   attachments:
 services:
   db:
@@ -54,10 +55,12 @@ services:
       - KINTO_PERMISSION_BACKEND=kinto.core.permission.postgresql
       - KINTO_PERMISSION_URL=postgresql://postgres@db/postgres
       - GRANIAN_ACCESS_LOG=true
+      - KINTO_SLACK_DEBUG_DIR=/app/slack
     volumes:
       - ./config:/app/config
       - ./kinto-remote-settings:/app/kinto-remote-settings
       - debug-mail:/app/mail
+      - debug-slack:/app/slack
       - attachments:/tmp/attachments
 
   git-reader:
@@ -82,8 +85,10 @@ services:
     environment:
       - SERVER=http://web:8888/v1
       - MAIL_DIR=/var/debug-mail/
+      - SLACK_DIR=/var/debug-slack/
     volumes:
       - debug-mail:/var/debug-mail/
+      - debug-slack:/var/debug-slack/
     shm_size: 2gb
   cronjobs:
     build:

kinto-remote-settings/src/kinto_remote_settings/signer/__init__.py

@@ -296,6 +296,15 @@ def on_new_request(event):
     except ImportError:  # pragma: no cover
         pass
 
+    try:
+        from kinto_slack import build_notification
+
+        config.add_subscriber(build_notification, ReviewRequested)
+        config.add_subscriber(build_notification, ReviewApproved)
+        config.add_subscriber(build_notification, ReviewRejected)
+    except ImportError:  # pragma: no cover
+        pass
+
     # Automatically create resources on startup if option is enabled.
     def auto_create_resources(event, resources):
         storage = event.app.registry.storage

kinto-slack/src/kinto_slack/__init__.py

@@ -1,5 +1,8 @@
+import json
 import logging
+import os
 import re
+import time
 from collections import defaultdict
 
 import requests
@@ -89,7 +92,8 @@ def build_notification(event):
         _context[resource_name + "_id"] = _context["id"] = obj["id"]
         messages += get_messages(storage, _context)
 
-    setattr(event.request, "_kinto_slack_messages", messages)
+    existing = getattr(event.request, "_kinto_slack_messages", [])
+    setattr(event.request, "_kinto_slack_messages", existing + messages)
 
 
 def send_notification(event):
@@ -99,16 +103,24 @@ def send_notification(event):
 
     settings = event.request.registry.settings
     webhook_url = settings.get("slack.webhook_url")
-    if not webhook_url:
+    debug_dir = settings.get("slack.debug_dir")
+
+    if not webhook_url and not debug_dir:
         logger.warning("slack.webhook_url is not configured")
         return
 
     for msg in messages:
-        try:
-            resp = requests.post(webhook_url, json=msg, timeout=5)
-            resp.raise_for_status()
-        except Exception:
-            logger.exception("Could not send Slack notification")
+        if debug_dir:
+            os.makedirs(debug_dir, exist_ok=True)
+            filename = os.path.join(debug_dir, f"{time.time_ns()}.json")
+            with open(filename, "w") as f:
+                json.dump(msg, f)
+        if webhook_url:
+            try:
+                resp = requests.post(webhook_url, json=msg, timeout=5)
+                resp.raise_for_status()
+            except Exception:
+                logger.exception("Could not send Slack notification")
 
 
 def _validate_slack_settings(event):
@@ -138,6 +150,10 @@ def includeme(config):
     webhook_url = read_env("kinto.slack.webhook_url", webhook_url)
     config.add_settings({"slack.webhook_url": webhook_url})
 
+    tmp_dir = settings.get("slack.debug_dir")
+    tmp_dir = read_env("kinto.slack.debug_dir", tmp_dir)
+    config.add_settings({"slack.debug_dir": tmp_dir})
+
     config.add_api_capability(
         "slack",
         "Slack notifications plugin for Kinto",