From a87ade4918e13a14bb2349b9d70a01e7c37c0002 Mon Sep 17 00:00:00 2001 From: Mr-Rahul-Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> Date: Thu, 25 Jun 2026 03:59:57 +0530 Subject: [PATCH 01/10] Move Slack moderation logic into a seperate service+ update test +read from yaml + new make target Signed-off-by: Mr-Rahul-Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> --- backend/.env.example | 3 + backend/src/apps/slack/MANIFEST.yaml | 2 + backend/src/apps/slack/Makefile | 4 + backend/src/apps/slack/admin/__init__.py | 1 + backend/src/apps/slack/admin/moderation.py | 47 +++++ .../config/moderation_rules.example.yaml | 9 + .../apps/slack/config/moderation_rules.yaml | 1 + backend/src/apps/slack/events/__init__.py | 1 + .../src/apps/slack/events/reaction_added.py | 14 ++ .../src/apps/slack/events/url_verification.py | 6 +- .../commands/slack_sync_moderation_rules.py | 114 +++++++++++ .../0023_moderationalert_moderationrule.py | 75 +++++++ backend/src/apps/slack/models/__init__.py | 1 + backend/src/apps/slack/models/moderation.py | 52 +++++ backend/src/apps/slack/services/moderation.py | 175 ++++++++++++++++ backend/src/settings/local.py | 15 +- .../unit/apps/slack/admin/moderation_test.py | 14 ++ .../slack_sync_moderation_rules_test.py | 182 +++++++++++++++++ .../apps/slack/events/reaction_added_test.py | 15 ++ .../slack/events/url_verification_test.py | 29 +-- .../unit/apps/slack/models/moderation_test.py | 13 ++ .../apps/slack/services/moderation_test.py | 191 ++++++++++++++++++ docker-compose/local/compose.yaml | 1 + 23 files changed, 938 insertions(+), 27 deletions(-) create mode 100644 backend/src/apps/slack/admin/moderation.py create mode 100644 backend/src/apps/slack/config/moderation_rules.example.yaml create mode 100644 backend/src/apps/slack/config/moderation_rules.yaml create mode 100644 backend/src/apps/slack/events/reaction_added.py create mode 100644 backend/src/apps/slack/management/commands/slack_sync_moderation_rules.py create mode 100644 backend/src/apps/slack/migrations/0023_moderationalert_moderationrule.py create mode 100644 backend/src/apps/slack/models/moderation.py create mode 100644 backend/src/apps/slack/services/moderation.py create mode 100644 backend/tests/unit/apps/slack/admin/moderation_test.py create mode 100644 backend/tests/unit/apps/slack/commands/slack_sync_moderation_rules_test.py create mode 100644 backend/tests/unit/apps/slack/events/reaction_added_test.py create mode 100644 backend/tests/unit/apps/slack/models/moderation_test.py create mode 100644 backend/tests/unit/apps/slack/services/moderation_test.py diff --git a/backend/.env.example b/backend/.env.example index f6408f67fa..12e1daa250 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -13,6 +13,8 @@ DJANGO_DB_USER=nest_user_dev DJANGO_ELEVENLABS_API_KEY=your-elevenlabs-api-key DJANGO_GITHUB_APP_ID=your-github-app-id DJANGO_GITHUB_APP_INSTALLATION_ID=your-github-app-installation-id +# Include the URL scheme, e.g. https://abc123.ngrok-free.app +DJANGO_LOCAL_NGROK_ORIGIN= DJANGO_OPEN_AI_SECRET_KEY=your-open-ai-secret-key DJANGO_PUBLIC_IP_ADDRESS=127.0.0.1 DJANGO_REDIS_AUTH_ENABLED=True @@ -26,6 +28,7 @@ DJANGO_SLACK_CLIENT_ID= DJANGO_SLACK_CLIENT_SECRET= DJANGO_SLACK_SEARCH_TOKEN= DJANGO_SLACK_SIGNING_SECRET= +SLACK_BOT_TOKEN_= GITHUB_TOKEN=your-github-token # Postgres image reads these on first volume init (must match DJANGO_DB_* above) diff --git a/backend/src/apps/slack/MANIFEST.yaml b/backend/src/apps/slack/MANIFEST.yaml index d5a3c28650..3fd8b605a2 100644 --- a/backend/src/apps/slack/MANIFEST.yaml +++ b/backend/src/apps/slack/MANIFEST.yaml @@ -121,6 +121,7 @@ oauth_config: - im:write - mpim:read - mpim:write + - reactions:read - reactions:write - users:read settings: @@ -134,6 +135,7 @@ settings: - app_mention - member_joined_channel - message.channels + - reaction_added - team_join interactivity: is_enabled: true diff --git a/backend/src/apps/slack/Makefile b/backend/src/apps/slack/Makefile index 0b9c775546..da6c751ccd 100644 --- a/backend/src/apps/slack/Makefile +++ b/backend/src/apps/slack/Makefile @@ -26,3 +26,7 @@ slack-sync-data: slack-sync-messages: @echo "Syncing Slack messages" @CMD="python manage.py slack_sync_messages" $(MAKE) exec-backend-command + +slack-sync-moderation-rules: + @echo "Syncing Slack moderation rules" + @CMD="python manage.py slack_sync_moderation_rules" $(MAKE) exec-backend-command diff --git a/backend/src/apps/slack/admin/__init__.py b/backend/src/apps/slack/admin/__init__.py index b3193ffb04..70300c17f5 100644 --- a/backend/src/apps/slack/admin/__init__.py +++ b/backend/src/apps/slack/admin/__init__.py @@ -4,4 +4,5 @@ from .event import EventAdmin from .member import MemberAdmin from .message import MessageAdmin +from .moderation import ModerationAlertAdmin, ModerationRuleAdmin from .workspace import WorkspaceAdmin diff --git a/backend/src/apps/slack/admin/moderation.py b/backend/src/apps/slack/admin/moderation.py new file mode 100644 index 0000000000..8ae9263d4a --- /dev/null +++ b/backend/src/apps/slack/admin/moderation.py @@ -0,0 +1,47 @@ +"""Django admin screens for Slack moderation configuration. + +Admins use rules to enable per-channel reaction thresholds, and alerts are +readonly-ish records showing which message/report pairs already notified. +""" + +from django.contrib import admin + +from apps.slack.models.moderation import ModerationAlert, ModerationRule + + +@admin.register(ModerationRule) +class ModerationRuleAdmin(admin.ModelAdmin): + """Admin list/search controls for moderation rules.""" + + list_display = ("conversation", "emoji_name", "report_type", "threshold", "is_enabled") + list_filter = ("is_enabled", "report_type") + search_fields = ("conversation__name", "emoji_name", "alert_channel_id") + + +@admin.register(ModerationAlert) +class ModerationAlertAdmin(admin.ModelAdmin): + """Admin list/search controls for emitted moderation alerts.""" + + list_display = ( + "conversation", + "message_ts", + "report_type", + "reaction_count", + "nest_created_at", + ) + search_fields = ("conversation__name", "message_ts", "report_type") + readonly_fields = ( + "conversation", + "message_ts", + "report_type", + "reaction_count", + "alert_message_ts", + ) + + def has_add_permission(self, request): + """Disable manual alert creation in Django admin.""" + return False + + def has_delete_permission(self, request, obj=None): + """Disable manual alert deletion in Django admin.""" + return False diff --git a/backend/src/apps/slack/config/moderation_rules.example.yaml b/backend/src/apps/slack/config/moderation_rules.example.yaml new file mode 100644 index 0000000000..4a52336c78 --- /dev/null +++ b/backend/src/apps/slack/config/moderation_rules.example.yaml @@ -0,0 +1,9 @@ +moderation_rules: + - channel_id: + emoji_name: + report_type: + threshold: + alert_channel_id: + alert_user_ids: + - + is_enabled: true diff --git a/backend/src/apps/slack/config/moderation_rules.yaml b/backend/src/apps/slack/config/moderation_rules.yaml new file mode 100644 index 0000000000..5c65734b63 --- /dev/null +++ b/backend/src/apps/slack/config/moderation_rules.yaml @@ -0,0 +1 @@ +moderation_rules: [] diff --git a/backend/src/apps/slack/events/__init__.py b/backend/src/apps/slack/events/__init__.py index 819a67a305..44e6c75808 100644 --- a/backend/src/apps/slack/events/__init__.py +++ b/backend/src/apps/slack/events/__init__.py @@ -5,6 +5,7 @@ def configure_slack_events(): app_home_opened, app_mention, message_posted, + reaction_added, team_join, url_verification, ) diff --git a/backend/src/apps/slack/events/reaction_added.py b/backend/src/apps/slack/events/reaction_added.py new file mode 100644 index 0000000000..c4ca75962f --- /dev/null +++ b/backend/src/apps/slack/events/reaction_added.py @@ -0,0 +1,14 @@ +"""Handle Slack reaction_added events.""" + +from apps.slack.events.event import EventBase +from apps.slack.services.moderation import process_reaction_added + + +class ReactionAdded(EventBase): + """Route reaction_added to moderation service for processing.""" + + event_type = "reaction_added" + + def handle_event(self, event, client): + """Handle report reactions added to Slack messages.""" + process_reaction_added(event, client) diff --git a/backend/src/apps/slack/events/url_verification.py b/backend/src/apps/slack/events/url_verification.py index 858d59544b..234e8c3267 100644 --- a/backend/src/apps/slack/events/url_verification.py +++ b/backend/src/apps/slack/events/url_verification.py @@ -8,6 +8,6 @@ class UrlVerification(EventBase): event_type = "url_verification" - def handle_event(self, event, client): - """Handle the URL verification event.""" - return event["challenge"] + def handler(self, event, client, ack): + """Acknowledge Slack URL verification challenges.""" + ack(event["challenge"]) diff --git a/backend/src/apps/slack/management/commands/slack_sync_moderation_rules.py b/backend/src/apps/slack/management/commands/slack_sync_moderation_rules.py new file mode 100644 index 0000000000..2c04b4aca1 --- /dev/null +++ b/backend/src/apps/slack/management/commands/slack_sync_moderation_rules.py @@ -0,0 +1,114 @@ +"""Sync Slack moderation rules from YAML config.""" + +from pathlib import Path + +import yaml +from django.conf import settings +from django.core.management.base import BaseCommand, CommandError + +from apps.slack.models import Conversation +from apps.slack.models.moderation import ModerationRule + +DEFAULT_CONFIG_PATH = Path(settings.BASE_DIR) / "apps/slack/config/moderation_rules.yaml" + +REQUIRED_FIELDS = { + "channel_id", + "emoji_name", + "report_type", + "threshold", + "alert_channel_id", +} + + +class Command(BaseCommand): + """Sync Slack moderation rules from a YAML file.""" + + help = "Sync Slack moderation rules from YAML config" + + def add_arguments(self, parser): + """Define command line arguments.""" + parser.add_argument( + "--config", + default=str(DEFAULT_CONFIG_PATH), + help="Path to the Slack moderation rules YAML file", + ) + + def handle(self, *args, **options): + """Sync moderation rules from YAML.""" + config_path = Path(options["config"]) + + if not config_path.exists(): + self.stdout.write(self.style.WARNING(f"Moderation config not found: {config_path}")) + return + + with config_path.open() as config_file: + config = yaml.safe_load(config_file) + + if config is None: + self.stdout.write(self.style.WARNING("Moderation config has no rules")) + return + + if not isinstance(config, dict): + msg = "Moderation config must be a mapping" + raise CommandError(msg) + + rules = config.get("moderation_rules", []) + if not isinstance(rules, list): + msg = "Moderation config must contain a moderation_rules list" + raise CommandError(msg) + + for rule in rules: + self._validate_rule(rule) + + synced_count = 0 + skipped_count = 0 + + for rule in rules: + try: + conversation = Conversation.objects.get(slack_channel_id=rule["channel_id"]) + except Conversation.DoesNotExist: + skipped_count += 1 + self.stdout.write( + self.style.WARNING( + f"Skipping moderation rule for missing channel: {rule['channel_id']}" + ) + ) + continue + + ModerationRule.objects.update_or_create( + conversation=conversation, + emoji_name=rule["emoji_name"], + defaults={ + "report_type": rule["report_type"], + "threshold": rule["threshold"], + "alert_channel_id": rule["alert_channel_id"], + "alert_user_ids": rule.get("alert_user_ids", []), + "is_enabled": rule.get("is_enabled", True), + }, + ) + synced_count += 1 + + self.stdout.write( + self.style.SUCCESS( + f"Synced {synced_count} moderation rule(s), skipped {skipped_count}" + ) + ) + + def _validate_rule(self, rule): + """Validate one moderation rule config entry.""" + if not isinstance(rule, dict): + msg = "Each moderation rule must be a mapping" + raise CommandError(msg) + + missing_fields = REQUIRED_FIELDS - rule.keys() + if missing_fields: + msg = f"Moderation rule missing required fields: {', '.join(sorted(missing_fields))}" + raise CommandError(msg) + + if not isinstance(rule["threshold"], int): + msg = "Moderation rule threshold must be an integer" + raise CommandError(msg) + + if rule["threshold"] < 1: + msg = "Moderation rule threshold must be at least 1" + raise CommandError(msg) diff --git a/backend/src/apps/slack/migrations/0023_moderationalert_moderationrule.py b/backend/src/apps/slack/migrations/0023_moderationalert_moderationrule.py new file mode 100644 index 0000000000..1754f4f498 --- /dev/null +++ b/backend/src/apps/slack/migrations/0023_moderationalert_moderationrule.py @@ -0,0 +1,75 @@ +# Generated by Django 6.0.6 on 2026-06-22 20:33 + +import django.core.validators +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("slack", "0022_workspace_invite_link_last_alert_message_ts_and_more"), + ] + + operations = [ + migrations.CreateModel( + name="ModerationAlert", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ("nest_created_at", models.DateTimeField(auto_now_add=True)), + ("nest_updated_at", models.DateTimeField(auto_now=True)), + ("message_ts", models.CharField(max_length=32)), + ("report_type", models.CharField(max_length=64)), + ("reaction_count", models.PositiveSmallIntegerField(default=0)), + ("alert_message_ts", models.CharField(blank=True, default="", max_length=32)), + ( + "conversation", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, to="slack.conversation" + ), + ), + ], + options={ + "db_table": "slack_moderation_alerts", + "unique_together": {("conversation", "message_ts", "report_type")}, + }, + ), + migrations.CreateModel( + name="ModerationRule", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ("nest_created_at", models.DateTimeField(auto_now_add=True)), + ("nest_updated_at", models.DateTimeField(auto_now=True)), + ("emoji_name", models.CharField(max_length=64)), + ("report_type", models.CharField(max_length=64)), + ( + "threshold", + models.PositiveSmallIntegerField( + default=3, validators=[django.core.validators.MinValueValidator(1)] + ), + ), + ("alert_channel_id", models.CharField(max_length=50)), + ("alert_user_ids", models.JSONField(blank=True, default=list)), + ("is_enabled", models.BooleanField(default=True)), + ( + "conversation", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, to="slack.conversation" + ), + ), + ], + options={ + "db_table": "slack_moderation_rules", + "unique_together": {("conversation", "emoji_name")}, + }, + ), + ] diff --git a/backend/src/apps/slack/models/__init__.py b/backend/src/apps/slack/models/__init__.py index 3bbe0878de..d6e6412785 100644 --- a/backend/src/apps/slack/models/__init__.py +++ b/backend/src/apps/slack/models/__init__.py @@ -2,4 +2,5 @@ from .event import Event from .member import Member from .message import Message +from .moderation import ModerationAlert, ModerationRule from .workspace import Workspace diff --git a/backend/src/apps/slack/models/moderation.py b/backend/src/apps/slack/models/moderation.py new file mode 100644 index 0000000000..c5f0b65f3c --- /dev/null +++ b/backend/src/apps/slack/models/moderation.py @@ -0,0 +1,52 @@ +"""Store Slack moderation rules and sent-alert records. + +ModerationRule configures which channel/emoji reaches which threshold. +ModerationAlert prevents sending duplicate moderator alerts for the same +message and report type. +""" + +from typing import override + +from django.core.validators import MinValueValidator +from django.db import models + +from apps.common.models import TimestampedModel +from apps.slack.models.conversation import Conversation + + +class ModerationRule(TimestampedModel): + """Channel-specific emoji threshold and alert target.""" + + class Meta: + """Model options.""" + + db_table = "slack_moderation_rules" + unique_together = ("conversation", "emoji_name") + + conversation = models.ForeignKey(Conversation, on_delete=models.CASCADE) + emoji_name = models.CharField(max_length=64) + report_type = models.CharField(max_length=64) + threshold = models.PositiveSmallIntegerField(default=3, validators=[MinValueValidator(1)]) + alert_channel_id = models.CharField(max_length=50) + alert_user_ids = models.JSONField(blank=True, default=list) + is_enabled = models.BooleanField(default=True) + + @override + def __str__(self): + return f"{self.conversation} :{self.emoji_name}" + + +class ModerationAlert(TimestampedModel): + """Record that an alert was already sent for a reported message.""" + + class Meta: + """Model options.""" + + db_table = "slack_moderation_alerts" + unique_together = ("conversation", "message_ts", "report_type") + + conversation = models.ForeignKey(Conversation, on_delete=models.CASCADE) + message_ts = models.CharField(max_length=32) + report_type = models.CharField(max_length=64) + reaction_count = models.PositiveSmallIntegerField(default=0) + alert_message_ts = models.CharField(max_length=32, blank=True, default="") diff --git a/backend/src/apps/slack/services/moderation.py b/backend/src/apps/slack/services/moderation.py new file mode 100644 index 0000000000..7b9a639b4b --- /dev/null +++ b/backend/src/apps/slack/services/moderation.py @@ -0,0 +1,175 @@ +"""Slack moderation services.""" + +import logging + +from django.db import IntegrityError +from slack_sdk.errors import SlackApiError + +from apps.slack.blocks import markdown +from apps.slack.models import Conversation +from apps.slack.models.moderation import ModerationAlert, ModerationRule + +logger = logging.getLogger(__name__) + + +def process_reaction_added(event, client): + """Process Slack reaction_added events for moderation alerts.""" + details = _get_message_reaction_details(event) + if details is None: + return + + channel_id, message_ts, emoji_name = details + conversation, rule = _get_moderation_rule(channel_id, emoji_name) + if rule is None: + return + + reaction_count = _get_reaction_count(client, channel_id, message_ts, emoji_name) + if reaction_count < rule.threshold: + return + + moderation_alert, created = _get_or_create_alert( + conversation, + message_ts, + rule.report_type, + reaction_count, + ) + if not created: + return + + permalink = _get_permalink(client, channel_id, message_ts) + if not permalink: + moderation_alert.delete() + return + + text = _build_alert_text(rule, channel_id, emoji_name, reaction_count, permalink) + alert = _post_alert(client, rule.alert_channel_id, text) + if alert is None: + moderation_alert.delete() + return + + _update_alert_message_ts(moderation_alert, alert) + + +def _get_message_reaction_details(event): + """Extract message reaction details from a Slack event.""" + item = event.get("item", {}) + if item.get("type") != "message": + return None + + channel_id = item.get("channel") + message_ts = item.get("ts") + emoji_name = event.get("reaction") + + if not channel_id or not message_ts or not emoji_name: + return None + + return channel_id, message_ts, emoji_name + + +def _get_moderation_rule(channel_id, emoji_name): + """Get the matching conversation and moderation rule, if configured.""" + try: + conversation = Conversation.objects.get(slack_channel_id=channel_id) + rule = ModerationRule.objects.get( + conversation=conversation, + emoji_name=emoji_name, + is_enabled=True, + ) + except (Conversation.DoesNotExist, ModerationRule.DoesNotExist): + return None, None + + return conversation, rule + + +def _get_reaction_count(client, channel_id, message_ts, emoji_name): + """Get the current unique user count for a reaction.""" + try: + response = client.reactions_get(channel=channel_id, timestamp=message_ts) + except SlackApiError as e: + logger.warning( + "Could not fetch Slack reactions for moderation: %s", + e.response.get("error", "unknown_error"), + ) + return 0 + + message = response.get("message", {}) + for reaction in message.get("reactions", []): + if reaction.get("name") == emoji_name: + return reaction.get("count", len(set(reaction.get("users", [])))) + + return 0 + + +def _get_or_create_alert(conversation, message_ts, report_type, reaction_count): + """Atomically claim the moderation alert before posting to Slack.""" + for _attempt in range(2): + try: + return ModerationAlert.objects.get_or_create( + conversation=conversation, + message_ts=message_ts, + report_type=report_type, + defaults={"reaction_count": reaction_count}, + ) + except IntegrityError: + try: + return ( + ModerationAlert.objects.get( + conversation=conversation, + message_ts=message_ts, + report_type=report_type, + ), + False, + ) + except ModerationAlert.DoesNotExist: + continue + + return None, False + + +def _get_permalink(client, channel_id, message_ts): + """Get the Slack permalink for the reported message.""" + try: + return client.chat_getPermalink( + channel=channel_id, + message_ts=message_ts, + )["permalink"] + except SlackApiError as e: + logger.warning( + "Could not fetch Slack permalink for moderation alert: %s", + e.response.get("error", "unknown_error"), + ) + return "" + + +def _post_alert(client, channel_id, text): + """Post a moderation alert to Slack.""" + try: + return client.chat_postMessage( + channel=channel_id, + blocks=[markdown(text)], + text=text, + ) + except SlackApiError as e: + logger.warning( + "Could not post Slack moderation alert: %s", + e.response.get("error", "unknown_error"), + ) + return None + + +def _update_alert_message_ts(moderation_alert, alert): + """Record the Slack notification timestamp on the claimed alert.""" + moderation_alert.alert_message_ts = alert.get("ts", "") + moderation_alert.save(update_fields=["alert_message_ts"]) + + +def _build_alert_text(rule, channel_id, emoji_name, reaction_count, permalink): + """Build the Slack moderation alert message.""" + mentions = " ".join(f"<@{user_id}>" for user_id in rule.alert_user_ids or []) + + return ( + f"{mentions}\n" + f":{emoji_name}: {rule.report_type} report threshold reached in <#{channel_id}>.\n" + f"Count: {reaction_count}\n" + f"{permalink}" + ).strip() diff --git a/backend/src/settings/local.py b/backend/src/settings/local.py index 914fcbecb3..2b82254c5b 100644 --- a/backend/src/settings/local.py +++ b/backend/src/settings/local.py @@ -1,18 +1,27 @@ """OWASP Nest local configuration.""" +import os + from configurations import values from settings.base import Base +LOCAL_NGROK_ORIGIN = os.environ.get("DJANGO_LOCAL_NGROK_ORIGIN", "") + class Local(Base): """Local configuration.""" APP_NAME = "OWASP Nest Local" - ALLOWED_ORIGINS = ( - "http://127.0.0.1:3000", - "http://localhost:3000", + ALLOWED_ORIGINS = tuple( + origin + for origin in ( + LOCAL_NGROK_ORIGIN, + "http://127.0.0.1:3000", + "http://localhost:3000", + ) + if origin ) CORS_ALLOWED_ORIGINS = ALLOWED_ORIGINS CSRF_TRUSTED_ORIGINS = ALLOWED_ORIGINS diff --git a/backend/tests/unit/apps/slack/admin/moderation_test.py b/backend/tests/unit/apps/slack/admin/moderation_test.py new file mode 100644 index 0000000000..49b70a38d2 --- /dev/null +++ b/backend/tests/unit/apps/slack/admin/moderation_test.py @@ -0,0 +1,14 @@ +from django.contrib.admin.sites import AdminSite + +from apps.slack.admin.moderation import ModerationAlertAdmin +from apps.slack.models.moderation import ModerationAlert + + +class TestModerationAlertAdmin: + def test_alerts_are_read_only_records(self): + """Test moderation alert records cannot be manually changed in admin.""" + admin = ModerationAlertAdmin(model=ModerationAlert, admin_site=AdminSite()) + + assert not admin.has_add_permission(request=None) + assert not admin.has_delete_permission(request=None) + assert "alert_message_ts" in admin.readonly_fields diff --git a/backend/tests/unit/apps/slack/commands/slack_sync_moderation_rules_test.py b/backend/tests/unit/apps/slack/commands/slack_sync_moderation_rules_test.py new file mode 100644 index 0000000000..2591a52f58 --- /dev/null +++ b/backend/tests/unit/apps/slack/commands/slack_sync_moderation_rules_test.py @@ -0,0 +1,182 @@ +"""Tests for the slack_sync_moderation_rules management command.""" + +from io import StringIO +from unittest.mock import Mock + +import pytest +from django.core.management.base import CommandError + +from apps.slack.management.commands.slack_sync_moderation_rules import Command +from apps.slack.models import Conversation + + +class TestSlackSyncModerationRulesCommand: + """Test cases for the slack_sync_moderation_rules management command.""" + + def test_missing_config_warns_and_continues(self, tmp_path): + """Test missing config skips moderation rule sync without failing startup.""" + command = Command() + command.stdout = StringIO() + + command.handle(config=str(tmp_path / "missing.yaml")) + + assert "Moderation config not found" in command.stdout.getvalue() + + def test_comment_only_config_warns_and_continues(self, tmp_path): + """Test comment-only config skips moderation rule sync without failing startup.""" + config_path = tmp_path / "moderation_rules.yaml" + config_path.write_text("# moderation_rules:\n# - channel_id: C_SOURCE\n") + + command = Command() + command.stdout = StringIO() + + command.handle(config=str(config_path)) + + assert "Moderation config has no rules" in command.stdout.getvalue() + + def test_missing_conversation_warns_and_skips_rule(self, mocker, tmp_path): + """Test rules for unknown channels are skipped without failing sync.""" + config_path = tmp_path / "moderation_rules.yaml" + config_path.write_text( + """ +moderation_rules: + - channel_id: C_UNKNOWN + emoji_name: spam + report_type: spam + threshold: 3 + alert_channel_id: C_MODERATION +""" + ) + mocker.patch( + "apps.slack.management.commands.slack_sync_moderation_rules.Conversation.objects.get", + side_effect=Conversation.DoesNotExist, + ) + update_or_create = mocker.patch( + "apps.slack.management.commands.slack_sync_moderation_rules." + "ModerationRule.objects.update_or_create" + ) + + command = Command() + command.stdout = StringIO() + + command.handle(config=str(config_path)) + + output = command.stdout.getvalue() + assert "Skipping moderation rule for missing channel: C_UNKNOWN" in output + update_or_create.assert_not_called() + + def test_valid_rule_updates_moderation_rule(self, mocker, tmp_path): + """Test a valid YAML rule creates or updates a moderation rule.""" + config_path = tmp_path / "moderation_rules.yaml" + config_path.write_text( + """ +moderation_rules: + - channel_id: C_SOURCE + emoji_name: spam + report_type: spam + threshold: 3 + alert_channel_id: C_MODERATION + alert_user_ids: + - U_MOD + is_enabled: true +""" + ) + conversation = Mock() + mocker.patch( + "apps.slack.management.commands.slack_sync_moderation_rules.Conversation.objects.get", + return_value=conversation, + ) + update_or_create = mocker.patch( + "apps.slack.management.commands.slack_sync_moderation_rules." + "ModerationRule.objects.update_or_create" + ) + + command = Command() + command.stdout = StringIO() + + command.handle(config=str(config_path)) + + update_or_create.assert_called_once_with( + conversation=conversation, + emoji_name="spam", + defaults={ + "report_type": "spam", + "threshold": 3, + "alert_channel_id": "C_MODERATION", + "alert_user_ids": ["U_MOD"], + "is_enabled": True, + }, + ) + assert "Synced 1 moderation rule(s), skipped 0" in command.stdout.getvalue() + + def test_non_mapping_config_raises_command_error(self, tmp_path): + """Test non-mapping YAML config fails with a clear command error.""" + config_path = tmp_path / "moderation_rules.yaml" + config_path.write_text("- invalid\n") + + with pytest.raises(CommandError, match="Moderation config must be a mapping"): + Command().handle(config=str(config_path)) + + def test_non_list_rules_config_raises_command_error(self, tmp_path): + """Test moderation_rules must be a list.""" + config_path = tmp_path / "moderation_rules.yaml" + config_path.write_text("moderation_rules: invalid\n") + + with pytest.raises(CommandError, match="moderation_rules list"): + Command().handle(config=str(config_path)) + + def test_missing_required_rule_field_raises_command_error(self, tmp_path): + """Test malformed rules fail before syncing any data.""" + config_path = tmp_path / "moderation_rules.yaml" + config_path.write_text( + """ +moderation_rules: + - channel_id: C_SOURCE + emoji_name: spam +""" + ) + + with pytest.raises(CommandError, match="missing required fields"): + Command().handle(config=str(config_path)) + + def test_non_integer_threshold_raises_command_error(self, tmp_path): + """Test non-integer thresholds fail with a clear command error.""" + config_path = tmp_path / "moderation_rules.yaml" + config_path.write_text( + """ +moderation_rules: + - channel_id: C_SOURCE + emoji_name: spam + report_type: spam + threshold: high + alert_channel_id: C_MODERATION +""" + ) + + with pytest.raises(CommandError, match="threshold must be an integer"): + Command().handle(config=str(config_path)) + + def test_invalid_rule_stops_before_partial_sync(self, mocker, tmp_path): + """Test all rules are validated before any moderation rule is synced.""" + config_path = tmp_path / "moderation_rules.yaml" + config_path.write_text( + """ +moderation_rules: + - channel_id: C_SOURCE + emoji_name: spam + report_type: spam + threshold: 3 + alert_channel_id: C_MODERATION + - channel_id: C_BROKEN + emoji_name: abusive +""" + ) + update_or_create = mocker.patch( + "apps.slack.management.commands.slack_sync_moderation_rules." + "ModerationRule.objects.update_or_create" + ) + + with pytest.raises(CommandError, match="missing required fields"): + Command().handle(config=str(config_path)) + + update_or_create.assert_not_called() diff --git a/backend/tests/unit/apps/slack/events/reaction_added_test.py b/backend/tests/unit/apps/slack/events/reaction_added_test.py new file mode 100644 index 0000000000..145246339e --- /dev/null +++ b/backend/tests/unit/apps/slack/events/reaction_added_test.py @@ -0,0 +1,15 @@ +from unittest.mock import Mock + +from apps.slack.events.reaction_added import ReactionAdded + + +class TestReactionAdded: + def test_handle_event_delegates_to_moderation_service(self, mocker): + """Test reaction_added events delegate moderation processing to the service.""" + service = mocker.patch("apps.slack.events.reaction_added.process_reaction_added") + event = {"type": "reaction_added"} + client = Mock() + + ReactionAdded().handle_event(event, client) + + service.assert_called_once_with(event, client) diff --git a/backend/tests/unit/apps/slack/events/url_verification_test.py b/backend/tests/unit/apps/slack/events/url_verification_test.py index d74910ac6d..41d05d707d 100644 --- a/backend/tests/unit/apps/slack/events/url_verification_test.py +++ b/backend/tests/unit/apps/slack/events/url_verification_test.py @@ -1,28 +1,15 @@ -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock -import pytest - -from apps.slack.apps import SlackConfig from apps.slack.events.url_verification import UrlVerification -@pytest.fixture -def mock_slack_app(): - mock_app = MagicMock() - mock_app.event = MagicMock() - return mock_app - - -@pytest.fixture -def slack_bot(mock_slack_app): - """Provide mocked SlackConfig.app, restored after the test.""" - with patch.object(SlackConfig, "app", mock_slack_app): - yield SlackConfig - - class TestUrlVerification: - def test_url_verification_handler(self, slack_bot): + def test_url_verification_handler_acknowledges_challenge(self): + """Test Slack URL verification responds with the challenge value.""" event = {"challenge": "test_challenge"} + ack = MagicMock() handler = UrlVerification() - response = handler.handle_event(event, client=None) - assert response == "test_challenge" + + handler.handler(event, client=None, ack=ack) + + ack.assert_called_once_with("test_challenge") diff --git a/backend/tests/unit/apps/slack/models/moderation_test.py b/backend/tests/unit/apps/slack/models/moderation_test.py new file mode 100644 index 0000000000..f6406319a9 --- /dev/null +++ b/backend/tests/unit/apps/slack/models/moderation_test.py @@ -0,0 +1,13 @@ +from apps.slack.models.conversation import Conversation +from apps.slack.models.moderation import ModerationRule +from apps.slack.models.workspace import Workspace + + +class TestModerationRule: + def test_str(self): + """Test moderation rule string includes conversation and emoji.""" + workspace = Workspace(name="OWASP") + conversation = Conversation(name="general", workspace=workspace) + rule = ModerationRule(conversation=conversation, emoji_name="spam") + + assert str(rule) == "OWASP #general :spam" diff --git a/backend/tests/unit/apps/slack/services/moderation_test.py b/backend/tests/unit/apps/slack/services/moderation_test.py new file mode 100644 index 0000000000..81aae8e9ef --- /dev/null +++ b/backend/tests/unit/apps/slack/services/moderation_test.py @@ -0,0 +1,191 @@ +from unittest.mock import Mock + +from django.db import IntegrityError +from slack_sdk.errors import SlackApiError + +from apps.slack.models.moderation import ModerationAlert +from apps.slack.services.moderation import _get_or_create_alert, process_reaction_added + +EVENT = { + "item": {"type": "message", "channel": "C_SOURCE", "ts": "123.000"}, + "reaction": "spam", +} + + +def slack_error(error="invalid_auth"): + """Build a Slack API error for service tests.""" + return SlackApiError(message="API error", response={"error": error}) + + +def mock_rule(threshold=1): + """Build a moderation rule mock.""" + return Mock( + alert_channel_id="C_ALERT", + alert_user_ids=["U_MOD"], + report_type="spam", + threshold=threshold, + ) + + +def mock_client(users=None): + """Build a Slack client mock with reaction and post responses.""" + client = Mock() + client.reactions_get.return_value = { + "message": { + "reactions": [ + { + "count": len(set(users or ["U1"])), + "name": "spam", + "users": users or ["U1"], + } + ] + } + } + client.chat_getPermalink.return_value = {"permalink": "https://slack.test/message"} + client.chat_postMessage.return_value = {"ts": "999.000"} + return client + + +def patch_rule_lookup(mocker, rule=None): + """Patch conversation and moderation rule lookups.""" + mocker.patch( + "apps.slack.services.moderation.Conversation.objects.get", + return_value=Mock(), + ) + mocker.patch( + "apps.slack.services.moderation.ModerationRule.objects.get", + return_value=rule or mock_rule(), + ) + + +class TestModerationService: + def test_process_reaction_added_posts_alert_and_records_it(self, mocker): + """Test threshold hit posts a Slack alert and records its message timestamp.""" + client = mock_client(users=["U1", "U2", "U1"]) + moderation_alert = Mock() + patch_rule_lookup(mocker, mock_rule(threshold=2)) + mocker.patch( + "apps.slack.services.moderation.ModerationAlert.objects.get_or_create", + return_value=(moderation_alert, True), + ) + + process_reaction_added(EVENT, client) + + client.reactions_get.assert_called_once_with(channel="C_SOURCE", timestamp="123.000") + client.chat_getPermalink.assert_called_once_with(channel="C_SOURCE", message_ts="123.000") + client.chat_postMessage.assert_called_once() + _, kwargs = client.chat_postMessage.call_args + assert kwargs["channel"] == "C_ALERT" + assert "<@U_MOD>" in kwargs["text"] + assert "spam report threshold reached" in kwargs["text"] + assert "Count: 2" in kwargs["text"] + assert "https://slack.test/message" in kwargs["text"] + assert moderation_alert.alert_message_ts == "999.000" + moderation_alert.save.assert_called_once_with(update_fields=["alert_message_ts"]) + + def test_process_reaction_added_skips_existing_alert(self, mocker): + """Test an existing moderation alert suppresses duplicate Slack posts.""" + client = mock_client() + moderation_alert = Mock() + patch_rule_lookup(mocker) + mocker.patch( + "apps.slack.services.moderation.ModerationAlert.objects.get_or_create", + return_value=(moderation_alert, False), + ) + + process_reaction_added(EVENT, client) + + client.chat_getPermalink.assert_not_called() + client.chat_postMessage.assert_not_called() + moderation_alert.save.assert_not_called() + + def test_process_reaction_added_stops_below_threshold(self, mocker): + """Test reactions below the configured threshold do not create alerts.""" + client = mock_client() + patch_rule_lookup(mocker, mock_rule(threshold=2)) + get_or_create = mocker.patch( + "apps.slack.services.moderation.ModerationAlert.objects.get_or_create" + ) + + process_reaction_added(EVENT, client) + + get_or_create.assert_not_called() + client.chat_postMessage.assert_not_called() + + def test_process_reaction_added_deletes_claimed_alert_when_post_fails(self, mocker): + """Test a claimed alert is deleted when Slack posting fails.""" + client = mock_client() + client.chat_postMessage.side_effect = slack_error("channel_not_found") + moderation_alert = Mock() + patch_rule_lookup(mocker) + mocker.patch( + "apps.slack.services.moderation.ModerationAlert.objects.get_or_create", + return_value=(moderation_alert, True), + ) + + process_reaction_added(EVENT, client) + + moderation_alert.delete.assert_called_once() + moderation_alert.save.assert_not_called() + + def test_process_reaction_added_deletes_claimed_alert_when_permalink_fails(self, mocker): + """Test a claimed alert is deleted when permalink lookup fails.""" + client = mock_client() + client.chat_getPermalink.side_effect = slack_error("message_not_found") + moderation_alert = Mock() + patch_rule_lookup(mocker) + mocker.patch( + "apps.slack.services.moderation.ModerationAlert.objects.get_or_create", + return_value=(moderation_alert, True), + ) + + process_reaction_added(EVENT, client) + + moderation_alert.delete.assert_called_once() + client.chat_postMessage.assert_not_called() + moderation_alert.save.assert_not_called() + + def test_process_reaction_added_stops_when_reactions_get_fails(self, mocker): + """Test Slack reaction API failures stop before claiming an alert.""" + client = Mock() + client.reactions_get.side_effect = slack_error() + patch_rule_lookup(mocker) + get_or_create = mocker.patch( + "apps.slack.services.moderation.ModerationAlert.objects.get_or_create" + ) + + process_reaction_added(EVENT, client) + + get_or_create.assert_not_called() + client.chat_postMessage.assert_not_called() + + def test_process_reaction_added_uses_slack_reaction_count(self, mocker): + """Test Slack reaction count is used when the users list is truncated.""" + client = mock_client(users=["U1"]) + client.reactions_get.return_value = { + "message": {"reactions": [{"count": 3, "name": "spam", "users": ["U1"]}]} + } + moderation_alert = Mock() + patch_rule_lookup(mocker, mock_rule(threshold=3)) + mocker.patch( + "apps.slack.services.moderation.ModerationAlert.objects.get_or_create", + return_value=(moderation_alert, True), + ) + + process_reaction_added(EVENT, client) + + _, kwargs = client.chat_postMessage.call_args + assert "Count: 3" in kwargs["text"] + + def test_get_or_create_alert_handles_deleted_race_fallback(self, mocker): + """Test IntegrityError fallback tolerates a concurrently deleted alert row.""" + manager = mocker.patch("apps.slack.services.moderation.ModerationAlert.objects") + manager.get_or_create.side_effect = IntegrityError + manager.get.side_effect = ModerationAlert.DoesNotExist + + alert, created = _get_or_create_alert(Mock(), "123.000", "spam", 1) + + assert alert is None + assert not created + assert manager.get_or_create.call_count == 2 + assert manager.get.call_count == 2 diff --git a/docker-compose/local/compose.yaml b/docker-compose/local/compose.yaml index 351c9139f4..c766357886 100644 --- a/docker-compose/local/compose.yaml +++ b/docker-compose/local/compose.yaml @@ -4,6 +4,7 @@ services: command: > sh -c ' python manage.py migrate && + python manage.py slack_sync_moderation_rules && python manage.py clear_cache && python manage.py runserver 0.0.0.0:8000 ' From 93f0df19bad4f24ed7f37cedc6437a786643da58 Mon Sep 17 00:00:00 2001 From: Mr-Rahul-Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> Date: Thu, 25 Jun 2026 04:36:26 +0530 Subject: [PATCH 02/10] Fix Slack workspace token placeholder Signed-off-by: Mr-Rahul-Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> --- backend/.env.example | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/.env.example b/backend/.env.example index 12e1daa250..323ae0f046 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -28,7 +28,8 @@ DJANGO_SLACK_CLIENT_ID= DJANGO_SLACK_CLIENT_SECRET= DJANGO_SLACK_SEARCH_TOKEN= DJANGO_SLACK_SIGNING_SECRET= -SLACK_BOT_TOKEN_= +# Replace T1234567890 with the actual Slack workspace ID. +SLACK_BOT_TOKEN_T1234567890= GITHUB_TOKEN=your-github-token # Postgres image reads these on first volume init (must match DJANGO_DB_* above) From 9e941af76f301a8b6fad21eb46c5dcad62179e2a Mon Sep 17 00:00:00 2001 From: Mr-Rahul-Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> Date: Thu, 25 Jun 2026 04:43:17 +0530 Subject: [PATCH 03/10] Add Slack moderation model help text Signed-off-by: Mr-Rahul-Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> --- .../0023_moderationalert_moderationrule.py | 50 ++++++++++++++++--- backend/src/apps/slack/models/moderation.py | 32 +++++++++--- 2 files changed, 70 insertions(+), 12 deletions(-) diff --git a/backend/src/apps/slack/migrations/0023_moderationalert_moderationrule.py b/backend/src/apps/slack/migrations/0023_moderationalert_moderationrule.py index 1754f4f498..a40e311ec8 100644 --- a/backend/src/apps/slack/migrations/0023_moderationalert_moderationrule.py +++ b/backend/src/apps/slack/migrations/0023_moderationalert_moderationrule.py @@ -22,10 +22,30 @@ class Migration(migrations.Migration): ), ("nest_created_at", models.DateTimeField(auto_now_add=True)), ("nest_updated_at", models.DateTimeField(auto_now=True)), - ("message_ts", models.CharField(max_length=32)), - ("report_type", models.CharField(max_length=64)), + ( + "message_ts", + models.CharField( + help_text="Slack timestamp of the message that triggered the alert.", + max_length=32, + ), + ), + ( + "report_type", + models.CharField( + help_text="Moderation report category for the emitted alert.", + max_length=64, + ), + ), ("reaction_count", models.PositiveSmallIntegerField(default=0)), - ("alert_message_ts", models.CharField(blank=True, default="", max_length=32)), + ( + "alert_message_ts", + models.CharField( + blank=True, + default="", + help_text="Slack timestamp of the posted moderation alert message.", + max_length=32, + ), + ), ( "conversation", models.ForeignKey( @@ -49,15 +69,33 @@ class Migration(migrations.Migration): ), ("nest_created_at", models.DateTimeField(auto_now_add=True)), ("nest_updated_at", models.DateTimeField(auto_now=True)), - ("emoji_name", models.CharField(max_length=64)), - ("report_type", models.CharField(max_length=64)), + ( + "emoji_name", + models.CharField( + help_text="Slack emoji name that triggers this moderation rule.", + max_length=64, + ), + ), + ( + "report_type", + models.CharField( + help_text=("Moderation report category recorded when this rule triggers."), + max_length=64, + ), + ), ( "threshold", models.PositiveSmallIntegerField( default=3, validators=[django.core.validators.MinValueValidator(1)] ), ), - ("alert_channel_id", models.CharField(max_length=50)), + ( + "alert_channel_id", + models.CharField( + help_text="Slack channel ID where moderation alerts are posted.", + max_length=50, + ), + ), ("alert_user_ids", models.JSONField(blank=True, default=list)), ("is_enabled", models.BooleanField(default=True)), ( diff --git a/backend/src/apps/slack/models/moderation.py b/backend/src/apps/slack/models/moderation.py index c5f0b65f3c..f84278756e 100644 --- a/backend/src/apps/slack/models/moderation.py +++ b/backend/src/apps/slack/models/moderation.py @@ -24,10 +24,19 @@ class Meta: unique_together = ("conversation", "emoji_name") conversation = models.ForeignKey(Conversation, on_delete=models.CASCADE) - emoji_name = models.CharField(max_length=64) - report_type = models.CharField(max_length=64) + emoji_name = models.CharField( + max_length=64, + help_text="Slack emoji name that triggers this moderation rule.", + ) + report_type = models.CharField( + max_length=64, + help_text="Moderation report category recorded when this rule triggers.", + ) threshold = models.PositiveSmallIntegerField(default=3, validators=[MinValueValidator(1)]) - alert_channel_id = models.CharField(max_length=50) + alert_channel_id = models.CharField( + max_length=50, + help_text="Slack channel ID where moderation alerts are posted.", + ) alert_user_ids = models.JSONField(blank=True, default=list) is_enabled = models.BooleanField(default=True) @@ -46,7 +55,18 @@ class Meta: unique_together = ("conversation", "message_ts", "report_type") conversation = models.ForeignKey(Conversation, on_delete=models.CASCADE) - message_ts = models.CharField(max_length=32) - report_type = models.CharField(max_length=64) + message_ts = models.CharField( + max_length=32, + help_text="Slack timestamp of the message that triggered the alert.", + ) + report_type = models.CharField( + max_length=64, + help_text="Moderation report category for the emitted alert.", + ) reaction_count = models.PositiveSmallIntegerField(default=0) - alert_message_ts = models.CharField(max_length=32, blank=True, default="") + alert_message_ts = models.CharField( + max_length=32, + blank=True, + default="", + help_text="Slack timestamp of the posted moderation alert message.", + ) From 4d68f8648c6b6b739b7c771e0c951ce503f94afd Mon Sep 17 00:00:00 2001 From: Mr-Rahul-Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> Date: Mon, 6 Jul 2026 12:06:36 +0530 Subject: [PATCH 04/10] Use admin-managed Slack reaction rules Signed-off-by: Mr-Rahul-Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> --- backend/src/apps/slack/Makefile | 4 - .../config/moderation_rules.example.yaml | 9 - .../apps/slack/config/moderation_rules.yaml | 1 - .../commands/slack_sync_moderation_rules.py | 114 ----------- .../slack_sync_moderation_rules_test.py | 182 ------------------ docker-compose/local/compose.yaml | 1 - 6 files changed, 311 deletions(-) delete mode 100644 backend/src/apps/slack/config/moderation_rules.example.yaml delete mode 100644 backend/src/apps/slack/config/moderation_rules.yaml delete mode 100644 backend/src/apps/slack/management/commands/slack_sync_moderation_rules.py delete mode 100644 backend/tests/unit/apps/slack/commands/slack_sync_moderation_rules_test.py diff --git a/backend/src/apps/slack/Makefile b/backend/src/apps/slack/Makefile index da6c751ccd..0b9c775546 100644 --- a/backend/src/apps/slack/Makefile +++ b/backend/src/apps/slack/Makefile @@ -26,7 +26,3 @@ slack-sync-data: slack-sync-messages: @echo "Syncing Slack messages" @CMD="python manage.py slack_sync_messages" $(MAKE) exec-backend-command - -slack-sync-moderation-rules: - @echo "Syncing Slack moderation rules" - @CMD="python manage.py slack_sync_moderation_rules" $(MAKE) exec-backend-command diff --git a/backend/src/apps/slack/config/moderation_rules.example.yaml b/backend/src/apps/slack/config/moderation_rules.example.yaml deleted file mode 100644 index 4a52336c78..0000000000 --- a/backend/src/apps/slack/config/moderation_rules.example.yaml +++ /dev/null @@ -1,9 +0,0 @@ -moderation_rules: - - channel_id: - emoji_name: - report_type: - threshold: - alert_channel_id: - alert_user_ids: - - - is_enabled: true diff --git a/backend/src/apps/slack/config/moderation_rules.yaml b/backend/src/apps/slack/config/moderation_rules.yaml deleted file mode 100644 index 5c65734b63..0000000000 --- a/backend/src/apps/slack/config/moderation_rules.yaml +++ /dev/null @@ -1 +0,0 @@ -moderation_rules: [] diff --git a/backend/src/apps/slack/management/commands/slack_sync_moderation_rules.py b/backend/src/apps/slack/management/commands/slack_sync_moderation_rules.py deleted file mode 100644 index 2c04b4aca1..0000000000 --- a/backend/src/apps/slack/management/commands/slack_sync_moderation_rules.py +++ /dev/null @@ -1,114 +0,0 @@ -"""Sync Slack moderation rules from YAML config.""" - -from pathlib import Path - -import yaml -from django.conf import settings -from django.core.management.base import BaseCommand, CommandError - -from apps.slack.models import Conversation -from apps.slack.models.moderation import ModerationRule - -DEFAULT_CONFIG_PATH = Path(settings.BASE_DIR) / "apps/slack/config/moderation_rules.yaml" - -REQUIRED_FIELDS = { - "channel_id", - "emoji_name", - "report_type", - "threshold", - "alert_channel_id", -} - - -class Command(BaseCommand): - """Sync Slack moderation rules from a YAML file.""" - - help = "Sync Slack moderation rules from YAML config" - - def add_arguments(self, parser): - """Define command line arguments.""" - parser.add_argument( - "--config", - default=str(DEFAULT_CONFIG_PATH), - help="Path to the Slack moderation rules YAML file", - ) - - def handle(self, *args, **options): - """Sync moderation rules from YAML.""" - config_path = Path(options["config"]) - - if not config_path.exists(): - self.stdout.write(self.style.WARNING(f"Moderation config not found: {config_path}")) - return - - with config_path.open() as config_file: - config = yaml.safe_load(config_file) - - if config is None: - self.stdout.write(self.style.WARNING("Moderation config has no rules")) - return - - if not isinstance(config, dict): - msg = "Moderation config must be a mapping" - raise CommandError(msg) - - rules = config.get("moderation_rules", []) - if not isinstance(rules, list): - msg = "Moderation config must contain a moderation_rules list" - raise CommandError(msg) - - for rule in rules: - self._validate_rule(rule) - - synced_count = 0 - skipped_count = 0 - - for rule in rules: - try: - conversation = Conversation.objects.get(slack_channel_id=rule["channel_id"]) - except Conversation.DoesNotExist: - skipped_count += 1 - self.stdout.write( - self.style.WARNING( - f"Skipping moderation rule for missing channel: {rule['channel_id']}" - ) - ) - continue - - ModerationRule.objects.update_or_create( - conversation=conversation, - emoji_name=rule["emoji_name"], - defaults={ - "report_type": rule["report_type"], - "threshold": rule["threshold"], - "alert_channel_id": rule["alert_channel_id"], - "alert_user_ids": rule.get("alert_user_ids", []), - "is_enabled": rule.get("is_enabled", True), - }, - ) - synced_count += 1 - - self.stdout.write( - self.style.SUCCESS( - f"Synced {synced_count} moderation rule(s), skipped {skipped_count}" - ) - ) - - def _validate_rule(self, rule): - """Validate one moderation rule config entry.""" - if not isinstance(rule, dict): - msg = "Each moderation rule must be a mapping" - raise CommandError(msg) - - missing_fields = REQUIRED_FIELDS - rule.keys() - if missing_fields: - msg = f"Moderation rule missing required fields: {', '.join(sorted(missing_fields))}" - raise CommandError(msg) - - if not isinstance(rule["threshold"], int): - msg = "Moderation rule threshold must be an integer" - raise CommandError(msg) - - if rule["threshold"] < 1: - msg = "Moderation rule threshold must be at least 1" - raise CommandError(msg) diff --git a/backend/tests/unit/apps/slack/commands/slack_sync_moderation_rules_test.py b/backend/tests/unit/apps/slack/commands/slack_sync_moderation_rules_test.py deleted file mode 100644 index 2591a52f58..0000000000 --- a/backend/tests/unit/apps/slack/commands/slack_sync_moderation_rules_test.py +++ /dev/null @@ -1,182 +0,0 @@ -"""Tests for the slack_sync_moderation_rules management command.""" - -from io import StringIO -from unittest.mock import Mock - -import pytest -from django.core.management.base import CommandError - -from apps.slack.management.commands.slack_sync_moderation_rules import Command -from apps.slack.models import Conversation - - -class TestSlackSyncModerationRulesCommand: - """Test cases for the slack_sync_moderation_rules management command.""" - - def test_missing_config_warns_and_continues(self, tmp_path): - """Test missing config skips moderation rule sync without failing startup.""" - command = Command() - command.stdout = StringIO() - - command.handle(config=str(tmp_path / "missing.yaml")) - - assert "Moderation config not found" in command.stdout.getvalue() - - def test_comment_only_config_warns_and_continues(self, tmp_path): - """Test comment-only config skips moderation rule sync without failing startup.""" - config_path = tmp_path / "moderation_rules.yaml" - config_path.write_text("# moderation_rules:\n# - channel_id: C_SOURCE\n") - - command = Command() - command.stdout = StringIO() - - command.handle(config=str(config_path)) - - assert "Moderation config has no rules" in command.stdout.getvalue() - - def test_missing_conversation_warns_and_skips_rule(self, mocker, tmp_path): - """Test rules for unknown channels are skipped without failing sync.""" - config_path = tmp_path / "moderation_rules.yaml" - config_path.write_text( - """ -moderation_rules: - - channel_id: C_UNKNOWN - emoji_name: spam - report_type: spam - threshold: 3 - alert_channel_id: C_MODERATION -""" - ) - mocker.patch( - "apps.slack.management.commands.slack_sync_moderation_rules.Conversation.objects.get", - side_effect=Conversation.DoesNotExist, - ) - update_or_create = mocker.patch( - "apps.slack.management.commands.slack_sync_moderation_rules." - "ModerationRule.objects.update_or_create" - ) - - command = Command() - command.stdout = StringIO() - - command.handle(config=str(config_path)) - - output = command.stdout.getvalue() - assert "Skipping moderation rule for missing channel: C_UNKNOWN" in output - update_or_create.assert_not_called() - - def test_valid_rule_updates_moderation_rule(self, mocker, tmp_path): - """Test a valid YAML rule creates or updates a moderation rule.""" - config_path = tmp_path / "moderation_rules.yaml" - config_path.write_text( - """ -moderation_rules: - - channel_id: C_SOURCE - emoji_name: spam - report_type: spam - threshold: 3 - alert_channel_id: C_MODERATION - alert_user_ids: - - U_MOD - is_enabled: true -""" - ) - conversation = Mock() - mocker.patch( - "apps.slack.management.commands.slack_sync_moderation_rules.Conversation.objects.get", - return_value=conversation, - ) - update_or_create = mocker.patch( - "apps.slack.management.commands.slack_sync_moderation_rules." - "ModerationRule.objects.update_or_create" - ) - - command = Command() - command.stdout = StringIO() - - command.handle(config=str(config_path)) - - update_or_create.assert_called_once_with( - conversation=conversation, - emoji_name="spam", - defaults={ - "report_type": "spam", - "threshold": 3, - "alert_channel_id": "C_MODERATION", - "alert_user_ids": ["U_MOD"], - "is_enabled": True, - }, - ) - assert "Synced 1 moderation rule(s), skipped 0" in command.stdout.getvalue() - - def test_non_mapping_config_raises_command_error(self, tmp_path): - """Test non-mapping YAML config fails with a clear command error.""" - config_path = tmp_path / "moderation_rules.yaml" - config_path.write_text("- invalid\n") - - with pytest.raises(CommandError, match="Moderation config must be a mapping"): - Command().handle(config=str(config_path)) - - def test_non_list_rules_config_raises_command_error(self, tmp_path): - """Test moderation_rules must be a list.""" - config_path = tmp_path / "moderation_rules.yaml" - config_path.write_text("moderation_rules: invalid\n") - - with pytest.raises(CommandError, match="moderation_rules list"): - Command().handle(config=str(config_path)) - - def test_missing_required_rule_field_raises_command_error(self, tmp_path): - """Test malformed rules fail before syncing any data.""" - config_path = tmp_path / "moderation_rules.yaml" - config_path.write_text( - """ -moderation_rules: - - channel_id: C_SOURCE - emoji_name: spam -""" - ) - - with pytest.raises(CommandError, match="missing required fields"): - Command().handle(config=str(config_path)) - - def test_non_integer_threshold_raises_command_error(self, tmp_path): - """Test non-integer thresholds fail with a clear command error.""" - config_path = tmp_path / "moderation_rules.yaml" - config_path.write_text( - """ -moderation_rules: - - channel_id: C_SOURCE - emoji_name: spam - report_type: spam - threshold: high - alert_channel_id: C_MODERATION -""" - ) - - with pytest.raises(CommandError, match="threshold must be an integer"): - Command().handle(config=str(config_path)) - - def test_invalid_rule_stops_before_partial_sync(self, mocker, tmp_path): - """Test all rules are validated before any moderation rule is synced.""" - config_path = tmp_path / "moderation_rules.yaml" - config_path.write_text( - """ -moderation_rules: - - channel_id: C_SOURCE - emoji_name: spam - report_type: spam - threshold: 3 - alert_channel_id: C_MODERATION - - channel_id: C_BROKEN - emoji_name: abusive -""" - ) - update_or_create = mocker.patch( - "apps.slack.management.commands.slack_sync_moderation_rules." - "ModerationRule.objects.update_or_create" - ) - - with pytest.raises(CommandError, match="missing required fields"): - Command().handle(config=str(config_path)) - - update_or_create.assert_not_called() diff --git a/docker-compose/local/compose.yaml b/docker-compose/local/compose.yaml index c766357886..351c9139f4 100644 --- a/docker-compose/local/compose.yaml +++ b/docker-compose/local/compose.yaml @@ -4,7 +4,6 @@ services: command: > sh -c ' python manage.py migrate && - python manage.py slack_sync_moderation_rules && python manage.py clear_cache && python manage.py runserver 0.0.0.0:8000 ' From cdc506dc2d830bd62410ea148fbb9cf6224793be Mon Sep 17 00:00:00 2001 From: Mr-Rahul-Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> Date: Mon, 6 Jul 2026 12:13:04 +0530 Subject: [PATCH 05/10] Generalize Slack moderation models as reactions Signed-off-by: Mr-Rahul-Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> --- backend/src/apps/slack/admin/__init__.py | 3 +- backend/src/apps/slack/admin/moderation.py | 47 ------------ .../src/apps/slack/admin/reaction_alert.py | 34 +++++++++ backend/src/apps/slack/admin/reaction_rule.py | 14 ++++ .../src/apps/slack/events/reaction_added.py | 4 +- ....py => 0023_reactionalert_reactionrule.py} | 18 ++--- backend/src/apps/slack/models/__init__.py | 3 +- backend/src/apps/slack/models/moderation.py | 72 ------------------- .../src/apps/slack/models/reaction_alert.py | 33 +++++++++ .../src/apps/slack/models/reaction_rule.py | 40 +++++++++++ .../services/{moderation.py => reaction.py} | 57 +++++++-------- .../unit/apps/slack/admin/moderation_test.py | 14 ---- .../apps/slack/admin/reaction_alert_test.py | 14 ++++ ...deration_test.py => reaction_rule_test.py} | 8 +-- .../{moderation_test.py => reaction_test.py} | 70 +++++++++--------- 15 files changed, 218 insertions(+), 213 deletions(-) delete mode 100644 backend/src/apps/slack/admin/moderation.py create mode 100644 backend/src/apps/slack/admin/reaction_alert.py create mode 100644 backend/src/apps/slack/admin/reaction_rule.py rename backend/src/apps/slack/migrations/{0023_moderationalert_moderationrule.py => 0023_reactionalert_reactionrule.py} (87%) delete mode 100644 backend/src/apps/slack/models/moderation.py create mode 100644 backend/src/apps/slack/models/reaction_alert.py create mode 100644 backend/src/apps/slack/models/reaction_rule.py rename backend/src/apps/slack/services/{moderation.py => reaction.py} (70%) delete mode 100644 backend/tests/unit/apps/slack/admin/moderation_test.py create mode 100644 backend/tests/unit/apps/slack/admin/reaction_alert_test.py rename backend/tests/unit/apps/slack/models/{moderation_test.py => reaction_rule_test.py} (56%) rename backend/tests/unit/apps/slack/services/{moderation_test.py => reaction_test.py} (71%) diff --git a/backend/src/apps/slack/admin/__init__.py b/backend/src/apps/slack/admin/__init__.py index 70300c17f5..a5bff42930 100644 --- a/backend/src/apps/slack/admin/__init__.py +++ b/backend/src/apps/slack/admin/__init__.py @@ -4,5 +4,6 @@ from .event import EventAdmin from .member import MemberAdmin from .message import MessageAdmin -from .moderation import ModerationAlertAdmin, ModerationRuleAdmin +from .reaction_alert import ReactionAlertAdmin +from .reaction_rule import ReactionRuleAdmin from .workspace import WorkspaceAdmin diff --git a/backend/src/apps/slack/admin/moderation.py b/backend/src/apps/slack/admin/moderation.py deleted file mode 100644 index 8ae9263d4a..0000000000 --- a/backend/src/apps/slack/admin/moderation.py +++ /dev/null @@ -1,47 +0,0 @@ -"""Django admin screens for Slack moderation configuration. - -Admins use rules to enable per-channel reaction thresholds, and alerts are -readonly-ish records showing which message/report pairs already notified. -""" - -from django.contrib import admin - -from apps.slack.models.moderation import ModerationAlert, ModerationRule - - -@admin.register(ModerationRule) -class ModerationRuleAdmin(admin.ModelAdmin): - """Admin list/search controls for moderation rules.""" - - list_display = ("conversation", "emoji_name", "report_type", "threshold", "is_enabled") - list_filter = ("is_enabled", "report_type") - search_fields = ("conversation__name", "emoji_name", "alert_channel_id") - - -@admin.register(ModerationAlert) -class ModerationAlertAdmin(admin.ModelAdmin): - """Admin list/search controls for emitted moderation alerts.""" - - list_display = ( - "conversation", - "message_ts", - "report_type", - "reaction_count", - "nest_created_at", - ) - search_fields = ("conversation__name", "message_ts", "report_type") - readonly_fields = ( - "conversation", - "message_ts", - "report_type", - "reaction_count", - "alert_message_ts", - ) - - def has_add_permission(self, request): - """Disable manual alert creation in Django admin.""" - return False - - def has_delete_permission(self, request, obj=None): - """Disable manual alert deletion in Django admin.""" - return False diff --git a/backend/src/apps/slack/admin/reaction_alert.py b/backend/src/apps/slack/admin/reaction_alert.py new file mode 100644 index 0000000000..db83a7fafe --- /dev/null +++ b/backend/src/apps/slack/admin/reaction_alert.py @@ -0,0 +1,34 @@ +"""Django admin screen for emitted Slack reaction alerts.""" + +from django.contrib import admin + +from apps.slack.models.reaction_alert import ReactionAlert + + +@admin.register(ReactionAlert) +class ReactionAlertAdmin(admin.ModelAdmin): + """Admin list/search controls for emitted reaction alerts.""" + + list_display = ( + "conversation", + "message_ts", + "report_type", + "reaction_count", + "nest_created_at", + ) + search_fields = ("conversation__name", "message_ts", "report_type") + readonly_fields = ( + "conversation", + "message_ts", + "report_type", + "reaction_count", + "alert_message_ts", + ) + + def has_add_permission(self, request): + """Disable manual alert creation in Django admin.""" + return False + + def has_delete_permission(self, request, obj=None): + """Disable manual alert deletion in Django admin.""" + return False diff --git a/backend/src/apps/slack/admin/reaction_rule.py b/backend/src/apps/slack/admin/reaction_rule.py new file mode 100644 index 0000000000..2354cd2396 --- /dev/null +++ b/backend/src/apps/slack/admin/reaction_rule.py @@ -0,0 +1,14 @@ +"""Django admin screen for Slack reaction rules.""" + +from django.contrib import admin + +from apps.slack.models.reaction_rule import ReactionRule + + +@admin.register(ReactionRule) +class ReactionRuleAdmin(admin.ModelAdmin): + """Admin list/search controls for reaction rules.""" + + list_display = ("conversation", "emoji_name", "report_type", "threshold", "is_enabled") + list_filter = ("is_enabled", "report_type") + search_fields = ("conversation__name", "emoji_name", "alert_channel_id") diff --git a/backend/src/apps/slack/events/reaction_added.py b/backend/src/apps/slack/events/reaction_added.py index c4ca75962f..774dad8c83 100644 --- a/backend/src/apps/slack/events/reaction_added.py +++ b/backend/src/apps/slack/events/reaction_added.py @@ -1,11 +1,11 @@ """Handle Slack reaction_added events.""" from apps.slack.events.event import EventBase -from apps.slack.services.moderation import process_reaction_added +from apps.slack.services.reaction import process_reaction_added class ReactionAdded(EventBase): - """Route reaction_added to moderation service for processing.""" + """Route reaction_added to the reaction service for processing.""" event_type = "reaction_added" diff --git a/backend/src/apps/slack/migrations/0023_moderationalert_moderationrule.py b/backend/src/apps/slack/migrations/0023_reactionalert_reactionrule.py similarity index 87% rename from backend/src/apps/slack/migrations/0023_moderationalert_moderationrule.py rename to backend/src/apps/slack/migrations/0023_reactionalert_reactionrule.py index a40e311ec8..3c77241eb5 100644 --- a/backend/src/apps/slack/migrations/0023_moderationalert_moderationrule.py +++ b/backend/src/apps/slack/migrations/0023_reactionalert_reactionrule.py @@ -12,7 +12,7 @@ class Migration(migrations.Migration): operations = [ migrations.CreateModel( - name="ModerationAlert", + name="ReactionAlert", fields=[ ( "id", @@ -32,7 +32,7 @@ class Migration(migrations.Migration): ( "report_type", models.CharField( - help_text="Moderation report category for the emitted alert.", + help_text="Report category for the emitted reaction alert.", max_length=64, ), ), @@ -42,7 +42,7 @@ class Migration(migrations.Migration): models.CharField( blank=True, default="", - help_text="Slack timestamp of the posted moderation alert message.", + help_text="Slack timestamp of the posted reaction alert message.", max_length=32, ), ), @@ -54,12 +54,12 @@ class Migration(migrations.Migration): ), ], options={ - "db_table": "slack_moderation_alerts", + "db_table": "slack_reaction_alerts", "unique_together": {("conversation", "message_ts", "report_type")}, }, ), migrations.CreateModel( - name="ModerationRule", + name="ReactionRule", fields=[ ( "id", @@ -72,14 +72,14 @@ class Migration(migrations.Migration): ( "emoji_name", models.CharField( - help_text="Slack emoji name that triggers this moderation rule.", + help_text="Slack emoji name that triggers this reaction rule.", max_length=64, ), ), ( "report_type", models.CharField( - help_text=("Moderation report category recorded when this rule triggers."), + help_text="Report category recorded when this reaction rule triggers.", max_length=64, ), ), @@ -92,7 +92,7 @@ class Migration(migrations.Migration): ( "alert_channel_id", models.CharField( - help_text="Slack channel ID where moderation alerts are posted.", + help_text="Slack channel ID where reaction alerts are posted.", max_length=50, ), ), @@ -106,7 +106,7 @@ class Migration(migrations.Migration): ), ], options={ - "db_table": "slack_moderation_rules", + "db_table": "slack_reaction_rules", "unique_together": {("conversation", "emoji_name")}, }, ), diff --git a/backend/src/apps/slack/models/__init__.py b/backend/src/apps/slack/models/__init__.py index d6e6412785..d83cba8c25 100644 --- a/backend/src/apps/slack/models/__init__.py +++ b/backend/src/apps/slack/models/__init__.py @@ -2,5 +2,6 @@ from .event import Event from .member import Member from .message import Message -from .moderation import ModerationAlert, ModerationRule +from .reaction_alert import ReactionAlert +from .reaction_rule import ReactionRule from .workspace import Workspace diff --git a/backend/src/apps/slack/models/moderation.py b/backend/src/apps/slack/models/moderation.py deleted file mode 100644 index f84278756e..0000000000 --- a/backend/src/apps/slack/models/moderation.py +++ /dev/null @@ -1,72 +0,0 @@ -"""Store Slack moderation rules and sent-alert records. - -ModerationRule configures which channel/emoji reaches which threshold. -ModerationAlert prevents sending duplicate moderator alerts for the same -message and report type. -""" - -from typing import override - -from django.core.validators import MinValueValidator -from django.db import models - -from apps.common.models import TimestampedModel -from apps.slack.models.conversation import Conversation - - -class ModerationRule(TimestampedModel): - """Channel-specific emoji threshold and alert target.""" - - class Meta: - """Model options.""" - - db_table = "slack_moderation_rules" - unique_together = ("conversation", "emoji_name") - - conversation = models.ForeignKey(Conversation, on_delete=models.CASCADE) - emoji_name = models.CharField( - max_length=64, - help_text="Slack emoji name that triggers this moderation rule.", - ) - report_type = models.CharField( - max_length=64, - help_text="Moderation report category recorded when this rule triggers.", - ) - threshold = models.PositiveSmallIntegerField(default=3, validators=[MinValueValidator(1)]) - alert_channel_id = models.CharField( - max_length=50, - help_text="Slack channel ID where moderation alerts are posted.", - ) - alert_user_ids = models.JSONField(blank=True, default=list) - is_enabled = models.BooleanField(default=True) - - @override - def __str__(self): - return f"{self.conversation} :{self.emoji_name}" - - -class ModerationAlert(TimestampedModel): - """Record that an alert was already sent for a reported message.""" - - class Meta: - """Model options.""" - - db_table = "slack_moderation_alerts" - unique_together = ("conversation", "message_ts", "report_type") - - conversation = models.ForeignKey(Conversation, on_delete=models.CASCADE) - message_ts = models.CharField( - max_length=32, - help_text="Slack timestamp of the message that triggered the alert.", - ) - report_type = models.CharField( - max_length=64, - help_text="Moderation report category for the emitted alert.", - ) - reaction_count = models.PositiveSmallIntegerField(default=0) - alert_message_ts = models.CharField( - max_length=32, - blank=True, - default="", - help_text="Slack timestamp of the posted moderation alert message.", - ) diff --git a/backend/src/apps/slack/models/reaction_alert.py b/backend/src/apps/slack/models/reaction_alert.py new file mode 100644 index 0000000000..2328b5dca4 --- /dev/null +++ b/backend/src/apps/slack/models/reaction_alert.py @@ -0,0 +1,33 @@ +"""Store emitted Slack reaction alerts.""" + +from django.db import models + +from apps.common.models import TimestampedModel +from apps.slack.models.conversation import Conversation + + +class ReactionAlert(TimestampedModel): + """Record that an alert was already sent for a reported message.""" + + class Meta: + """Model options.""" + + db_table = "slack_reaction_alerts" + unique_together = ("conversation", "message_ts", "report_type") + + conversation = models.ForeignKey(Conversation, on_delete=models.CASCADE) + message_ts = models.CharField( + max_length=32, + help_text="Slack timestamp of the message that triggered the alert.", + ) + report_type = models.CharField( + max_length=64, + help_text="Report category for the emitted reaction alert.", + ) + reaction_count = models.PositiveSmallIntegerField(default=0) + alert_message_ts = models.CharField( + max_length=32, + blank=True, + default="", + help_text="Slack timestamp of the posted reaction alert message.", + ) diff --git a/backend/src/apps/slack/models/reaction_rule.py b/backend/src/apps/slack/models/reaction_rule.py new file mode 100644 index 0000000000..dd532b7b65 --- /dev/null +++ b/backend/src/apps/slack/models/reaction_rule.py @@ -0,0 +1,40 @@ +"""Store channel-specific Slack reaction rules.""" + +from typing import override + +from django.core.validators import MinValueValidator +from django.db import models + +from apps.common.models import TimestampedModel +from apps.slack.models.conversation import Conversation + + +class ReactionRule(TimestampedModel): + """Channel-specific emoji threshold and alert target.""" + + class Meta: + """Model options.""" + + db_table = "slack_reaction_rules" + unique_together = ("conversation", "emoji_name") + + conversation = models.ForeignKey(Conversation, on_delete=models.CASCADE) + emoji_name = models.CharField( + max_length=64, + help_text="Slack emoji name that triggers this reaction rule.", + ) + report_type = models.CharField( + max_length=64, + help_text="Report category recorded when this reaction rule triggers.", + ) + threshold = models.PositiveSmallIntegerField(default=3, validators=[MinValueValidator(1)]) + alert_channel_id = models.CharField( + max_length=50, + help_text="Slack channel ID where reaction alerts are posted.", + ) + alert_user_ids = models.JSONField(blank=True, default=list) + is_enabled = models.BooleanField(default=True) + + @override + def __str__(self): + return f"{self.conversation} :{self.emoji_name}" diff --git a/backend/src/apps/slack/services/moderation.py b/backend/src/apps/slack/services/reaction.py similarity index 70% rename from backend/src/apps/slack/services/moderation.py rename to backend/src/apps/slack/services/reaction.py index 7b9a639b4b..8a4d03e764 100644 --- a/backend/src/apps/slack/services/moderation.py +++ b/backend/src/apps/slack/services/reaction.py @@ -1,4 +1,4 @@ -"""Slack moderation services.""" +"""Slack reaction services.""" import logging @@ -7,27 +7,28 @@ from apps.slack.blocks import markdown from apps.slack.models import Conversation -from apps.slack.models.moderation import ModerationAlert, ModerationRule +from apps.slack.models.reaction_alert import ReactionAlert +from apps.slack.models.reaction_rule import ReactionRule logger = logging.getLogger(__name__) def process_reaction_added(event, client): """Process Slack reaction_added events for moderation alerts.""" - details = _get_message_reaction_details(event) + details = get_message_reaction_details(event) if details is None: return channel_id, message_ts, emoji_name = details - conversation, rule = _get_moderation_rule(channel_id, emoji_name) + conversation, rule = get_reaction_rule(channel_id, emoji_name) if rule is None: return - reaction_count = _get_reaction_count(client, channel_id, message_ts, emoji_name) + reaction_count = get_reaction_count(client, channel_id, message_ts, emoji_name) if reaction_count < rule.threshold: return - moderation_alert, created = _get_or_create_alert( + reaction_alert, created = get_or_create_alert( conversation, message_ts, rule.report_type, @@ -36,21 +37,21 @@ def process_reaction_added(event, client): if not created: return - permalink = _get_permalink(client, channel_id, message_ts) + permalink = get_permalink(client, channel_id, message_ts) if not permalink: - moderation_alert.delete() + reaction_alert.delete() return - text = _build_alert_text(rule, channel_id, emoji_name, reaction_count, permalink) - alert = _post_alert(client, rule.alert_channel_id, text) + text = build_alert_text(rule, channel_id, emoji_name, reaction_count, permalink) + alert = post_alert(client, rule.alert_channel_id, text) if alert is None: - moderation_alert.delete() + reaction_alert.delete() return - _update_alert_message_ts(moderation_alert, alert) + update_alert_message_ts(reaction_alert, alert) -def _get_message_reaction_details(event): +def get_message_reaction_details(event): """Extract message reaction details from a Slack event.""" item = event.get("item", {}) if item.get("type") != "message": @@ -66,22 +67,22 @@ def _get_message_reaction_details(event): return channel_id, message_ts, emoji_name -def _get_moderation_rule(channel_id, emoji_name): - """Get the matching conversation and moderation rule, if configured.""" +def get_reaction_rule(channel_id, emoji_name): + """Get the matching conversation and reaction rule, if configured.""" try: conversation = Conversation.objects.get(slack_channel_id=channel_id) - rule = ModerationRule.objects.get( + rule = ReactionRule.objects.get( conversation=conversation, emoji_name=emoji_name, is_enabled=True, ) - except (Conversation.DoesNotExist, ModerationRule.DoesNotExist): + except (Conversation.DoesNotExist, ReactionRule.DoesNotExist): return None, None return conversation, rule -def _get_reaction_count(client, channel_id, message_ts, emoji_name): +def get_reaction_count(client, channel_id, message_ts, emoji_name): """Get the current unique user count for a reaction.""" try: response = client.reactions_get(channel=channel_id, timestamp=message_ts) @@ -100,11 +101,11 @@ def _get_reaction_count(client, channel_id, message_ts, emoji_name): return 0 -def _get_or_create_alert(conversation, message_ts, report_type, reaction_count): +def get_or_create_alert(conversation, message_ts, report_type, reaction_count): """Atomically claim the moderation alert before posting to Slack.""" for _attempt in range(2): try: - return ModerationAlert.objects.get_or_create( + return ReactionAlert.objects.get_or_create( conversation=conversation, message_ts=message_ts, report_type=report_type, @@ -113,20 +114,20 @@ def _get_or_create_alert(conversation, message_ts, report_type, reaction_count): except IntegrityError: try: return ( - ModerationAlert.objects.get( + ReactionAlert.objects.get( conversation=conversation, message_ts=message_ts, report_type=report_type, ), False, ) - except ModerationAlert.DoesNotExist: + except ReactionAlert.DoesNotExist: continue return None, False -def _get_permalink(client, channel_id, message_ts): +def get_permalink(client, channel_id, message_ts): """Get the Slack permalink for the reported message.""" try: return client.chat_getPermalink( @@ -141,7 +142,7 @@ def _get_permalink(client, channel_id, message_ts): return "" -def _post_alert(client, channel_id, text): +def post_alert(client, channel_id, text): """Post a moderation alert to Slack.""" try: return client.chat_postMessage( @@ -157,13 +158,13 @@ def _post_alert(client, channel_id, text): return None -def _update_alert_message_ts(moderation_alert, alert): +def update_alert_message_ts(reaction_alert, alert): """Record the Slack notification timestamp on the claimed alert.""" - moderation_alert.alert_message_ts = alert.get("ts", "") - moderation_alert.save(update_fields=["alert_message_ts"]) + reaction_alert.alert_message_ts = alert.get("ts", "") + reaction_alert.save(update_fields=["alert_message_ts"]) -def _build_alert_text(rule, channel_id, emoji_name, reaction_count, permalink): +def build_alert_text(rule, channel_id, emoji_name, reaction_count, permalink): """Build the Slack moderation alert message.""" mentions = " ".join(f"<@{user_id}>" for user_id in rule.alert_user_ids or []) diff --git a/backend/tests/unit/apps/slack/admin/moderation_test.py b/backend/tests/unit/apps/slack/admin/moderation_test.py deleted file mode 100644 index 49b70a38d2..0000000000 --- a/backend/tests/unit/apps/slack/admin/moderation_test.py +++ /dev/null @@ -1,14 +0,0 @@ -from django.contrib.admin.sites import AdminSite - -from apps.slack.admin.moderation import ModerationAlertAdmin -from apps.slack.models.moderation import ModerationAlert - - -class TestModerationAlertAdmin: - def test_alerts_are_read_only_records(self): - """Test moderation alert records cannot be manually changed in admin.""" - admin = ModerationAlertAdmin(model=ModerationAlert, admin_site=AdminSite()) - - assert not admin.has_add_permission(request=None) - assert not admin.has_delete_permission(request=None) - assert "alert_message_ts" in admin.readonly_fields diff --git a/backend/tests/unit/apps/slack/admin/reaction_alert_test.py b/backend/tests/unit/apps/slack/admin/reaction_alert_test.py new file mode 100644 index 0000000000..893283c6c0 --- /dev/null +++ b/backend/tests/unit/apps/slack/admin/reaction_alert_test.py @@ -0,0 +1,14 @@ +from django.contrib.admin.sites import AdminSite + +from apps.slack.admin.reaction_alert import ReactionAlertAdmin +from apps.slack.models.reaction_alert import ReactionAlert + + +class TestReactionAlertAdmin: + def test_alerts_are_read_only_records(self): + """Test reaction alert records cannot be manually changed in admin.""" + admin = ReactionAlertAdmin(model=ReactionAlert, admin_site=AdminSite()) + + assert not admin.has_add_permission(request=None) + assert not admin.has_delete_permission(request=None) + assert "alert_message_ts" in admin.readonly_fields diff --git a/backend/tests/unit/apps/slack/models/moderation_test.py b/backend/tests/unit/apps/slack/models/reaction_rule_test.py similarity index 56% rename from backend/tests/unit/apps/slack/models/moderation_test.py rename to backend/tests/unit/apps/slack/models/reaction_rule_test.py index f6406319a9..bdbce7a414 100644 --- a/backend/tests/unit/apps/slack/models/moderation_test.py +++ b/backend/tests/unit/apps/slack/models/reaction_rule_test.py @@ -1,13 +1,13 @@ from apps.slack.models.conversation import Conversation -from apps.slack.models.moderation import ModerationRule +from apps.slack.models.reaction_rule import ReactionRule from apps.slack.models.workspace import Workspace -class TestModerationRule: +class TestReactionRule: def test_str(self): - """Test moderation rule string includes conversation and emoji.""" + """Test reaction rule string includes conversation and emoji.""" workspace = Workspace(name="OWASP") conversation = Conversation(name="general", workspace=workspace) - rule = ModerationRule(conversation=conversation, emoji_name="spam") + rule = ReactionRule(conversation=conversation, emoji_name="spam") assert str(rule) == "OWASP #general :spam" diff --git a/backend/tests/unit/apps/slack/services/moderation_test.py b/backend/tests/unit/apps/slack/services/reaction_test.py similarity index 71% rename from backend/tests/unit/apps/slack/services/moderation_test.py rename to backend/tests/unit/apps/slack/services/reaction_test.py index 81aae8e9ef..c37f72f704 100644 --- a/backend/tests/unit/apps/slack/services/moderation_test.py +++ b/backend/tests/unit/apps/slack/services/reaction_test.py @@ -3,8 +3,8 @@ from django.db import IntegrityError from slack_sdk.errors import SlackApiError -from apps.slack.models.moderation import ModerationAlert -from apps.slack.services.moderation import _get_or_create_alert, process_reaction_added +from apps.slack.models.reaction_alert import ReactionAlert +from apps.slack.services.reaction import get_or_create_alert, process_reaction_added EVENT = { "item": {"type": "message", "channel": "C_SOURCE", "ts": "123.000"}, @@ -18,7 +18,7 @@ def slack_error(error="invalid_auth"): def mock_rule(threshold=1): - """Build a moderation rule mock.""" + """Build a reaction rule mock.""" return Mock( alert_channel_id="C_ALERT", alert_user_ids=["U_MOD"], @@ -47,26 +47,26 @@ def mock_client(users=None): def patch_rule_lookup(mocker, rule=None): - """Patch conversation and moderation rule lookups.""" + """Patch conversation and reaction rule lookups.""" mocker.patch( - "apps.slack.services.moderation.Conversation.objects.get", + "apps.slack.services.reaction.Conversation.objects.get", return_value=Mock(), ) mocker.patch( - "apps.slack.services.moderation.ModerationRule.objects.get", + "apps.slack.services.reaction.ReactionRule.objects.get", return_value=rule or mock_rule(), ) -class TestModerationService: +class TestReactionService: def test_process_reaction_added_posts_alert_and_records_it(self, mocker): """Test threshold hit posts a Slack alert and records its message timestamp.""" client = mock_client(users=["U1", "U2", "U1"]) - moderation_alert = Mock() + reaction_alert = Mock() patch_rule_lookup(mocker, mock_rule(threshold=2)) mocker.patch( - "apps.slack.services.moderation.ModerationAlert.objects.get_or_create", - return_value=(moderation_alert, True), + "apps.slack.services.reaction.ReactionAlert.objects.get_or_create", + return_value=(reaction_alert, True), ) process_reaction_added(EVENT, client) @@ -80,31 +80,31 @@ def test_process_reaction_added_posts_alert_and_records_it(self, mocker): assert "spam report threshold reached" in kwargs["text"] assert "Count: 2" in kwargs["text"] assert "https://slack.test/message" in kwargs["text"] - assert moderation_alert.alert_message_ts == "999.000" - moderation_alert.save.assert_called_once_with(update_fields=["alert_message_ts"]) + assert reaction_alert.alert_message_ts == "999.000" + reaction_alert.save.assert_called_once_with(update_fields=["alert_message_ts"]) def test_process_reaction_added_skips_existing_alert(self, mocker): - """Test an existing moderation alert suppresses duplicate Slack posts.""" + """Test an existing reaction alert suppresses duplicate Slack posts.""" client = mock_client() - moderation_alert = Mock() + reaction_alert = Mock() patch_rule_lookup(mocker) mocker.patch( - "apps.slack.services.moderation.ModerationAlert.objects.get_or_create", - return_value=(moderation_alert, False), + "apps.slack.services.reaction.ReactionAlert.objects.get_or_create", + return_value=(reaction_alert, False), ) process_reaction_added(EVENT, client) client.chat_getPermalink.assert_not_called() client.chat_postMessage.assert_not_called() - moderation_alert.save.assert_not_called() + reaction_alert.save.assert_not_called() def test_process_reaction_added_stops_below_threshold(self, mocker): """Test reactions below the configured threshold do not create alerts.""" client = mock_client() patch_rule_lookup(mocker, mock_rule(threshold=2)) get_or_create = mocker.patch( - "apps.slack.services.moderation.ModerationAlert.objects.get_or_create" + "apps.slack.services.reaction.ReactionAlert.objects.get_or_create" ) process_reaction_added(EVENT, client) @@ -116,34 +116,34 @@ def test_process_reaction_added_deletes_claimed_alert_when_post_fails(self, mock """Test a claimed alert is deleted when Slack posting fails.""" client = mock_client() client.chat_postMessage.side_effect = slack_error("channel_not_found") - moderation_alert = Mock() + reaction_alert = Mock() patch_rule_lookup(mocker) mocker.patch( - "apps.slack.services.moderation.ModerationAlert.objects.get_or_create", - return_value=(moderation_alert, True), + "apps.slack.services.reaction.ReactionAlert.objects.get_or_create", + return_value=(reaction_alert, True), ) process_reaction_added(EVENT, client) - moderation_alert.delete.assert_called_once() - moderation_alert.save.assert_not_called() + reaction_alert.delete.assert_called_once() + reaction_alert.save.assert_not_called() def test_process_reaction_added_deletes_claimed_alert_when_permalink_fails(self, mocker): """Test a claimed alert is deleted when permalink lookup fails.""" client = mock_client() client.chat_getPermalink.side_effect = slack_error("message_not_found") - moderation_alert = Mock() + reaction_alert = Mock() patch_rule_lookup(mocker) mocker.patch( - "apps.slack.services.moderation.ModerationAlert.objects.get_or_create", - return_value=(moderation_alert, True), + "apps.slack.services.reaction.ReactionAlert.objects.get_or_create", + return_value=(reaction_alert, True), ) process_reaction_added(EVENT, client) - moderation_alert.delete.assert_called_once() + reaction_alert.delete.assert_called_once() client.chat_postMessage.assert_not_called() - moderation_alert.save.assert_not_called() + reaction_alert.save.assert_not_called() def test_process_reaction_added_stops_when_reactions_get_fails(self, mocker): """Test Slack reaction API failures stop before claiming an alert.""" @@ -151,7 +151,7 @@ def test_process_reaction_added_stops_when_reactions_get_fails(self, mocker): client.reactions_get.side_effect = slack_error() patch_rule_lookup(mocker) get_or_create = mocker.patch( - "apps.slack.services.moderation.ModerationAlert.objects.get_or_create" + "apps.slack.services.reaction.ReactionAlert.objects.get_or_create" ) process_reaction_added(EVENT, client) @@ -165,11 +165,11 @@ def test_process_reaction_added_uses_slack_reaction_count(self, mocker): client.reactions_get.return_value = { "message": {"reactions": [{"count": 3, "name": "spam", "users": ["U1"]}]} } - moderation_alert = Mock() + reaction_alert = Mock() patch_rule_lookup(mocker, mock_rule(threshold=3)) mocker.patch( - "apps.slack.services.moderation.ModerationAlert.objects.get_or_create", - return_value=(moderation_alert, True), + "apps.slack.services.reaction.ReactionAlert.objects.get_or_create", + return_value=(reaction_alert, True), ) process_reaction_added(EVENT, client) @@ -179,11 +179,11 @@ def test_process_reaction_added_uses_slack_reaction_count(self, mocker): def test_get_or_create_alert_handles_deleted_race_fallback(self, mocker): """Test IntegrityError fallback tolerates a concurrently deleted alert row.""" - manager = mocker.patch("apps.slack.services.moderation.ModerationAlert.objects") + manager = mocker.patch("apps.slack.services.reaction.ReactionAlert.objects") manager.get_or_create.side_effect = IntegrityError - manager.get.side_effect = ModerationAlert.DoesNotExist + manager.get.side_effect = ReactionAlert.DoesNotExist - alert, created = _get_or_create_alert(Mock(), "123.000", "spam", 1) + alert, created = get_or_create_alert(Mock(), "123.000", "spam", 1) assert alert is None assert not created From 1f867071c16e92dbf6f5a6909ac416c6c9bd904e Mon Sep 17 00:00:00 2001 From: Mr-Rahul-Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> Date: Mon, 6 Jul 2026 12:14:13 +0530 Subject: [PATCH 06/10] Remove unnecessary local ngrok origin setting Signed-off-by: Mr-Rahul-Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> --- backend/.env.example | 2 -- backend/src/settings/local.py | 16 +++------------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/backend/.env.example b/backend/.env.example index 323ae0f046..efb271937b 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -13,8 +13,6 @@ DJANGO_DB_USER=nest_user_dev DJANGO_ELEVENLABS_API_KEY=your-elevenlabs-api-key DJANGO_GITHUB_APP_ID=your-github-app-id DJANGO_GITHUB_APP_INSTALLATION_ID=your-github-app-installation-id -# Include the URL scheme, e.g. https://abc123.ngrok-free.app -DJANGO_LOCAL_NGROK_ORIGIN= DJANGO_OPEN_AI_SECRET_KEY=your-open-ai-secret-key DJANGO_PUBLIC_IP_ADDRESS=127.0.0.1 DJANGO_REDIS_AUTH_ENABLED=True diff --git a/backend/src/settings/local.py b/backend/src/settings/local.py index 2b82254c5b..c7307757f6 100644 --- a/backend/src/settings/local.py +++ b/backend/src/settings/local.py @@ -1,27 +1,17 @@ """OWASP Nest local configuration.""" -import os - from configurations import values from settings.base import Base -LOCAL_NGROK_ORIGIN = os.environ.get("DJANGO_LOCAL_NGROK_ORIGIN", "") - - class Local(Base): """Local configuration.""" APP_NAME = "OWASP Nest Local" - ALLOWED_ORIGINS = tuple( - origin - for origin in ( - LOCAL_NGROK_ORIGIN, - "http://127.0.0.1:3000", - "http://localhost:3000", - ) - if origin + ALLOWED_ORIGINS = ( + "http://127.0.0.1:3000", + "http://localhost:3000", ) CORS_ALLOWED_ORIGINS = ALLOWED_ORIGINS CSRF_TRUSTED_ORIGINS = ALLOWED_ORIGINS From 02acae1dffd90a3ff8405321ed538991e9acb42c Mon Sep 17 00:00:00 2001 From: Mr-Rahul-Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> Date: Mon, 6 Jul 2026 12:35:06 +0530 Subject: [PATCH 07/10] check test fixes Signed-off-by: Mr-Rahul-Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> --- backend/src/settings/local.py | 1 + 1 file changed, 1 insertion(+) diff --git a/backend/src/settings/local.py b/backend/src/settings/local.py index c7307757f6..914fcbecb3 100644 --- a/backend/src/settings/local.py +++ b/backend/src/settings/local.py @@ -4,6 +4,7 @@ from settings.base import Base + class Local(Base): """Local configuration.""" From baaa11b589e268a7ed4719f154f132ba1ae1005b Mon Sep 17 00:00:00 2001 From: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> Date: Sun, 16 Aug 2026 18:53:12 -0700 Subject: [PATCH 08/10] Update code Signed-off-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> --- backend/.env.example | 5 +- .../src/apps/slack/admin/reaction_alert.py | 7 +- backend/src/apps/slack/admin/reaction_rule.py | 20 +- .../src/apps/slack/events/reaction_added.py | 87 ++++++- .../0024_reactionalert_reporter_user_ids.py | 23 ++ .../0025_alter_reactionrule_report_type.py | 22 ++ .../0026_rename_reactionrule_is_enabled.py | 17 ++ .../0027_alter_reactionrule_emoji_name.py | 23 ++ .../src/apps/slack/models/reaction_alert.py | 82 ++++++- .../src/apps/slack/models/reaction_rule.py | 45 +++- backend/src/apps/slack/services/reaction.py | 176 ------------- backend/src/apps/slack/utils/reaction.py | 37 +++ .../apps/slack/admin/reaction_alert_test.py | 2 +- .../apps/slack/events/reaction_added_test.py | 231 +++++++++++++++++- .../apps/slack/models/reaction_alert_test.py | 118 +++++++++ .../apps/slack/models/reaction_rule_test.py | 30 +++ .../unit/apps/slack/services/reaction_test.py | 191 --------------- .../tests/unit/apps/slack/utils/__init__.py | 1 + .../unit/apps/slack/utils/reaction_test.py | 60 +++++ 19 files changed, 775 insertions(+), 402 deletions(-) create mode 100644 backend/src/apps/slack/migrations/0024_reactionalert_reporter_user_ids.py create mode 100644 backend/src/apps/slack/migrations/0025_alter_reactionrule_report_type.py create mode 100644 backend/src/apps/slack/migrations/0026_rename_reactionrule_is_enabled.py create mode 100644 backend/src/apps/slack/migrations/0027_alter_reactionrule_emoji_name.py delete mode 100644 backend/src/apps/slack/services/reaction.py create mode 100644 backend/src/apps/slack/utils/reaction.py create mode 100644 backend/tests/unit/apps/slack/models/reaction_alert_test.py delete mode 100644 backend/tests/unit/apps/slack/services/reaction_test.py create mode 100644 backend/tests/unit/apps/slack/utils/__init__.py create mode 100644 backend/tests/unit/apps/slack/utils/reaction_test.py diff --git a/backend/.env.example b/backend/.env.example index efb271937b..5e0ca57505 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -26,11 +26,12 @@ DJANGO_SLACK_CLIENT_ID= DJANGO_SLACK_CLIENT_SECRET= DJANGO_SLACK_SEARCH_TOKEN= DJANGO_SLACK_SIGNING_SECRET= -# Replace T1234567890 with the actual Slack workspace ID. -SLACK_BOT_TOKEN_T1234567890= GITHUB_TOKEN=your-github-token # Postgres image reads these on first volume init (must match DJANGO_DB_* above) POSTGRES_DB=nest_db_dev POSTGRES_PASSWORD=nest_user_dev_password POSTGRES_USER=nest_user_dev + +# Replace T1234567890 with the actual Slack workspace ID. +SLACK_BOT_TOKEN_T1234567890= diff --git a/backend/src/apps/slack/admin/reaction_alert.py b/backend/src/apps/slack/admin/reaction_alert.py index db83a7fafe..eebac240bc 100644 --- a/backend/src/apps/slack/admin/reaction_alert.py +++ b/backend/src/apps/slack/admin/reaction_alert.py @@ -16,14 +16,19 @@ class ReactionAlertAdmin(admin.ModelAdmin): "reaction_count", "nest_created_at", ) - search_fields = ("conversation__name", "message_ts", "report_type") readonly_fields = ( "conversation", "message_ts", "report_type", "reaction_count", + "reporter_user_ids", "alert_message_ts", ) + search_fields = ( + "conversation__name", + "message_ts", + "report_type", + ) def has_add_permission(self, request): """Disable manual alert creation in Django admin.""" diff --git a/backend/src/apps/slack/admin/reaction_rule.py b/backend/src/apps/slack/admin/reaction_rule.py index 2354cd2396..ce5251602f 100644 --- a/backend/src/apps/slack/admin/reaction_rule.py +++ b/backend/src/apps/slack/admin/reaction_rule.py @@ -9,6 +9,20 @@ class ReactionRuleAdmin(admin.ModelAdmin): """Admin list/search controls for reaction rules.""" - list_display = ("conversation", "emoji_name", "report_type", "threshold", "is_enabled") - list_filter = ("is_enabled", "report_type") - search_fields = ("conversation__name", "emoji_name", "alert_channel_id") + autocomplete_fields = ("conversation",) + list_display = ( + "conversation", + "emoji_name", + "report_type", + "threshold", + "is_active", + ) + list_filter = ( + "is_active", + "report_type", + ) + search_fields = ( + "conversation__name", + "emoji_name", + "alert_channel_id", + ) diff --git a/backend/src/apps/slack/events/reaction_added.py b/backend/src/apps/slack/events/reaction_added.py index 774dad8c83..31f99ac85a 100644 --- a/backend/src/apps/slack/events/reaction_added.py +++ b/backend/src/apps/slack/events/reaction_added.py @@ -1,14 +1,93 @@ """Handle Slack reaction_added events.""" +import logging + +from slack_sdk.errors import SlackApiError + +from apps.slack.blocks import markdown from apps.slack.events.event import EventBase -from apps.slack.services.reaction import process_reaction_added +from apps.slack.models.reaction_alert import ReactionAlert +from apps.slack.models.reaction_rule import ReactionRule +from apps.slack.utils.reaction import mention_users, parse_message_reaction, reaction_from_payload + +logger = logging.getLogger(__name__) + + +def fetch_reaction(client, channel_id: str, message_ts: str, emoji_name: str): + """Return Slack's current reaction snapshot for the emoji, or None.""" + try: + payload = client.reactions_get( + channel=channel_id, + full=True, + timestamp=message_ts, + ) + except SlackApiError as e: + logger.warning( + "Could not fetch Slack reactions for moderation alert: %s", + e.response.get("error", "unknown_error"), + ) + return None + return reaction_from_payload(payload, emoji_name) class ReactionAdded(EventBase): - """Route reaction_added to the reaction service for processing.""" + """Handle reaction_added events for moderation alerts.""" event_type = "reaction_added" def handle_event(self, event, client): - """Handle report reactions added to Slack messages.""" - process_reaction_added(event, client) + """Post an alert when Slack shows the rule threshold is reached.""" + if (details := parse_message_reaction(event)) is None: + return + + channel_id, message_ts, emoji_name = details + if (rule := ReactionRule.for_reaction(channel_id, emoji_name)) is None: + return + if ReactionAlert.exists_for(rule.conversation, message_ts, rule.report_type): + return + + if (snapshot := fetch_reaction(client, channel_id, message_ts, emoji_name)) is None: + return + + reaction_count, reporter_user_ids, permalink = snapshot + if reaction_count < rule.threshold or not permalink: + return + + # Lock in-flight posts; the DB row is written only after Slack succeeds. + if not ReactionAlert.acquire(rule.conversation, message_ts, rule.report_type): + return + + try: + alert_users = mention_users(rule.alert_user_ids) + reporters = mention_users(reporter_user_ids) + text = ( + f"{alert_users}\n" + f":{emoji_name}: A message in <#{channel_id}> reached the " + f"{rule.report_type} report threshold." + ) + if reporters: + text = f"{text}\nReported by: {reporters}" + text = f"{text}\n{permalink}".strip() + + try: + alert = client.chat_postMessage( + blocks=[markdown(text)], + channel=rule.alert_channel_id, + text=text, + ) + except SlackApiError as e: + logger.warning( + "Could not post Slack moderation alert: %s", + e.response.get("error", "unknown_error"), + ) + else: + ReactionAlert.record( + rule.conversation, + message_ts, + rule.report_type, + reaction_count, + alert.get("ts", ""), + reporter_user_ids=reporter_user_ids, + ) + finally: + ReactionAlert.release(rule.conversation, message_ts, rule.report_type) diff --git a/backend/src/apps/slack/migrations/0024_reactionalert_reporter_user_ids.py b/backend/src/apps/slack/migrations/0024_reactionalert_reporter_user_ids.py new file mode 100644 index 0000000000..98caf30bee --- /dev/null +++ b/backend/src/apps/slack/migrations/0024_reactionalert_reporter_user_ids.py @@ -0,0 +1,23 @@ +# Generated by Django 6.0.8 on 2026-08-17 00:17 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("slack", "0023_reactionalert_reactionrule"), + ] + + operations = [ + migrations.AddField( + model_name="reactionalert", + name="reporter_user_ids", + field=models.JSONField( + blank=True, + default=list, + help_text=( + "Slack user IDs that had the triggering reaction when the alert was posted." + ), + ), + ), + ] diff --git a/backend/src/apps/slack/migrations/0025_alter_reactionrule_report_type.py b/backend/src/apps/slack/migrations/0025_alter_reactionrule_report_type.py new file mode 100644 index 0000000000..17d009edce --- /dev/null +++ b/backend/src/apps/slack/migrations/0025_alter_reactionrule_report_type.py @@ -0,0 +1,22 @@ +# Generated by Django 6.0.8 on 2026-08-17 01:08 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("slack", "0024_reactionalert_reporter_user_ids"), + ] + + operations = [ + migrations.AlterField( + model_name="reactionrule", + name="report_type", + field=models.CharField( + choices=[("spam", "Spam")], + default="spam", + help_text="Report category recorded when this reaction rule triggers.", + max_length=64, + ), + ), + ] diff --git a/backend/src/apps/slack/migrations/0026_rename_reactionrule_is_enabled.py b/backend/src/apps/slack/migrations/0026_rename_reactionrule_is_enabled.py new file mode 100644 index 0000000000..8698ae0969 --- /dev/null +++ b/backend/src/apps/slack/migrations/0026_rename_reactionrule_is_enabled.py @@ -0,0 +1,17 @@ +# Generated by Django 6.0.8 on 2026-08-17 01:09 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("slack", "0025_alter_reactionrule_report_type"), + ] + + operations = [ + migrations.RenameField( + model_name="reactionrule", + old_name="is_enabled", + new_name="is_active", + ), + ] diff --git a/backend/src/apps/slack/migrations/0027_alter_reactionrule_emoji_name.py b/backend/src/apps/slack/migrations/0027_alter_reactionrule_emoji_name.py new file mode 100644 index 0000000000..ff94be025d --- /dev/null +++ b/backend/src/apps/slack/migrations/0027_alter_reactionrule_emoji_name.py @@ -0,0 +1,23 @@ +# Generated by Django 6.0.8 on 2026-08-17 01:28 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("slack", "0026_rename_reactionrule_is_enabled"), + ] + + operations = [ + migrations.AlterField( + model_name="reactionrule", + name="emoji_name", + field=models.CharField( + help_text=( + "Slack emoji name that triggers this reaction rule, without leading or " + "trailing colons (spam, not :spam:)." + ), + max_length=64, + ), + ), + ] diff --git a/backend/src/apps/slack/models/reaction_alert.py b/backend/src/apps/slack/models/reaction_alert.py index 2328b5dca4..62c3436863 100644 --- a/backend/src/apps/slack/models/reaction_alert.py +++ b/backend/src/apps/slack/models/reaction_alert.py @@ -1,10 +1,13 @@ """Store emitted Slack reaction alerts.""" -from django.db import models +from django.core.cache import cache +from django.db import IntegrityError, models from apps.common.models import TimestampedModel from apps.slack.models.conversation import Conversation +LOCK_TTL_SECONDS = 30 + class ReactionAlert(TimestampedModel): """Record that an alert was already sent for a reported message.""" @@ -15,19 +18,84 @@ class Meta: db_table = "slack_reaction_alerts" unique_together = ("conversation", "message_ts", "report_type") - conversation = models.ForeignKey(Conversation, on_delete=models.CASCADE) + alert_message_ts = models.CharField( + max_length=32, + blank=True, + default="", + help_text="Slack timestamp of the posted reaction alert message.", + ) message_ts = models.CharField( max_length=32, help_text="Slack timestamp of the message that triggered the alert.", ) + reaction_count = models.PositiveSmallIntegerField(default=0) report_type = models.CharField( max_length=64, help_text="Report category for the emitted reaction alert.", ) - reaction_count = models.PositiveSmallIntegerField(default=0) - alert_message_ts = models.CharField( - max_length=32, + reporter_user_ids = models.JSONField( blank=True, - default="", - help_text="Slack timestamp of the posted reaction alert message.", + default=list, + help_text="Slack user IDs that had the triggering reaction when the alert was posted.", ) + + # FKs. + conversation = models.ForeignKey(Conversation, on_delete=models.CASCADE) + + @staticmethod + def acquire(conversation: Conversation, message_ts: str, report_type: str) -> bool: + """Return True if this process should post the alert.""" + if ReactionAlert.exists_for(conversation, message_ts, report_type): + return False + + key = ReactionAlert.lock_key(conversation, message_ts, report_type) + if not cache.add(key, 1, timeout=LOCK_TTL_SECONDS): + return False + + if ReactionAlert.exists_for(conversation, message_ts, report_type): + cache.delete(key) + return False + + return True + + @staticmethod + def exists_for(conversation: Conversation, message_ts: str, report_type: str) -> bool: + """Return True if an alert was already recorded for this message.""" + return ReactionAlert.objects.filter( + conversation=conversation, + message_ts=message_ts, + report_type=report_type, + ).exists() + + @staticmethod + def lock_key(conversation: Conversation, message_ts: str, report_type: str) -> str: + """Return the cache key for an in-flight reaction alert.""" + return f"slack:reaction-alert:{conversation.pk}:{message_ts}:{report_type}" + + @staticmethod + def record( + conversation: Conversation, + message_ts: str, + report_type: str, + reaction_count: int, + alert_message_ts: str, + *, + reporter_user_ids: list[str], + ) -> None: + """Store that an alert was sent, ignoring a concurrent insert.""" + try: + ReactionAlert.objects.create( + alert_message_ts=alert_message_ts, + conversation=conversation, + message_ts=message_ts, + reaction_count=reaction_count, + report_type=report_type, + reporter_user_ids=reporter_user_ids, + ) + except IntegrityError: + return + + @staticmethod + def release(conversation: Conversation, message_ts: str, report_type: str) -> None: + """Release the in-flight lock so a later reaction can retry.""" + cache.delete(ReactionAlert.lock_key(conversation, message_ts, report_type)) diff --git a/backend/src/apps/slack/models/reaction_rule.py b/backend/src/apps/slack/models/reaction_rule.py index dd532b7b65..eceafbd282 100644 --- a/backend/src/apps/slack/models/reaction_rule.py +++ b/backend/src/apps/slack/models/reaction_rule.py @@ -1,7 +1,5 @@ """Store channel-specific Slack reaction rules.""" -from typing import override - from django.core.validators import MinValueValidator from django.db import models @@ -12,29 +10,54 @@ class ReactionRule(TimestampedModel): """Channel-specific emoji threshold and alert target.""" + class ReportType(models.TextChoices): + """Reaction report category choices.""" + + SPAM = "spam", "Spam" + class Meta: """Model options.""" db_table = "slack_reaction_rules" unique_together = ("conversation", "emoji_name") - conversation = models.ForeignKey(Conversation, on_delete=models.CASCADE) + alert_channel_id = models.CharField( + max_length=50, + help_text="Slack channel ID where reaction alerts are posted.", + ) + alert_user_ids = models.JSONField(blank=True, default=list) emoji_name = models.CharField( max_length=64, - help_text="Slack emoji name that triggers this reaction rule.", + help_text=( + "Slack emoji name that triggers this reaction rule, without leading or " + "trailing colons (spam, not :spam:)." + ), ) + is_active = models.BooleanField(default=True) report_type = models.CharField( max_length=64, + choices=ReportType.choices, + default=ReportType.SPAM, help_text="Report category recorded when this reaction rule triggers.", ) threshold = models.PositiveSmallIntegerField(default=3, validators=[MinValueValidator(1)]) - alert_channel_id = models.CharField( - max_length=50, - help_text="Slack channel ID where reaction alerts are posted.", - ) - alert_user_ids = models.JSONField(blank=True, default=list) - is_enabled = models.BooleanField(default=True) - @override + # FKs. + conversation = models.ForeignKey(Conversation, on_delete=models.CASCADE) + def __str__(self): + """Human readable representation.""" return f"{self.conversation} :{self.emoji_name}" + + @staticmethod + def for_reaction(channel_id: str, emoji_name: str) -> "ReactionRule | None": + """Get the active reaction rule for a channel and emoji, if configured.""" + return ( + ReactionRule.objects.select_related("conversation") + .filter( + conversation__slack_channel_id=channel_id, + emoji_name=emoji_name, + is_active=True, + ) + .first() + ) diff --git a/backend/src/apps/slack/services/reaction.py b/backend/src/apps/slack/services/reaction.py deleted file mode 100644 index 8a4d03e764..0000000000 --- a/backend/src/apps/slack/services/reaction.py +++ /dev/null @@ -1,176 +0,0 @@ -"""Slack reaction services.""" - -import logging - -from django.db import IntegrityError -from slack_sdk.errors import SlackApiError - -from apps.slack.blocks import markdown -from apps.slack.models import Conversation -from apps.slack.models.reaction_alert import ReactionAlert -from apps.slack.models.reaction_rule import ReactionRule - -logger = logging.getLogger(__name__) - - -def process_reaction_added(event, client): - """Process Slack reaction_added events for moderation alerts.""" - details = get_message_reaction_details(event) - if details is None: - return - - channel_id, message_ts, emoji_name = details - conversation, rule = get_reaction_rule(channel_id, emoji_name) - if rule is None: - return - - reaction_count = get_reaction_count(client, channel_id, message_ts, emoji_name) - if reaction_count < rule.threshold: - return - - reaction_alert, created = get_or_create_alert( - conversation, - message_ts, - rule.report_type, - reaction_count, - ) - if not created: - return - - permalink = get_permalink(client, channel_id, message_ts) - if not permalink: - reaction_alert.delete() - return - - text = build_alert_text(rule, channel_id, emoji_name, reaction_count, permalink) - alert = post_alert(client, rule.alert_channel_id, text) - if alert is None: - reaction_alert.delete() - return - - update_alert_message_ts(reaction_alert, alert) - - -def get_message_reaction_details(event): - """Extract message reaction details from a Slack event.""" - item = event.get("item", {}) - if item.get("type") != "message": - return None - - channel_id = item.get("channel") - message_ts = item.get("ts") - emoji_name = event.get("reaction") - - if not channel_id or not message_ts or not emoji_name: - return None - - return channel_id, message_ts, emoji_name - - -def get_reaction_rule(channel_id, emoji_name): - """Get the matching conversation and reaction rule, if configured.""" - try: - conversation = Conversation.objects.get(slack_channel_id=channel_id) - rule = ReactionRule.objects.get( - conversation=conversation, - emoji_name=emoji_name, - is_enabled=True, - ) - except (Conversation.DoesNotExist, ReactionRule.DoesNotExist): - return None, None - - return conversation, rule - - -def get_reaction_count(client, channel_id, message_ts, emoji_name): - """Get the current unique user count for a reaction.""" - try: - response = client.reactions_get(channel=channel_id, timestamp=message_ts) - except SlackApiError as e: - logger.warning( - "Could not fetch Slack reactions for moderation: %s", - e.response.get("error", "unknown_error"), - ) - return 0 - - message = response.get("message", {}) - for reaction in message.get("reactions", []): - if reaction.get("name") == emoji_name: - return reaction.get("count", len(set(reaction.get("users", [])))) - - return 0 - - -def get_or_create_alert(conversation, message_ts, report_type, reaction_count): - """Atomically claim the moderation alert before posting to Slack.""" - for _attempt in range(2): - try: - return ReactionAlert.objects.get_or_create( - conversation=conversation, - message_ts=message_ts, - report_type=report_type, - defaults={"reaction_count": reaction_count}, - ) - except IntegrityError: - try: - return ( - ReactionAlert.objects.get( - conversation=conversation, - message_ts=message_ts, - report_type=report_type, - ), - False, - ) - except ReactionAlert.DoesNotExist: - continue - - return None, False - - -def get_permalink(client, channel_id, message_ts): - """Get the Slack permalink for the reported message.""" - try: - return client.chat_getPermalink( - channel=channel_id, - message_ts=message_ts, - )["permalink"] - except SlackApiError as e: - logger.warning( - "Could not fetch Slack permalink for moderation alert: %s", - e.response.get("error", "unknown_error"), - ) - return "" - - -def post_alert(client, channel_id, text): - """Post a moderation alert to Slack.""" - try: - return client.chat_postMessage( - channel=channel_id, - blocks=[markdown(text)], - text=text, - ) - except SlackApiError as e: - logger.warning( - "Could not post Slack moderation alert: %s", - e.response.get("error", "unknown_error"), - ) - return None - - -def update_alert_message_ts(reaction_alert, alert): - """Record the Slack notification timestamp on the claimed alert.""" - reaction_alert.alert_message_ts = alert.get("ts", "") - reaction_alert.save(update_fields=["alert_message_ts"]) - - -def build_alert_text(rule, channel_id, emoji_name, reaction_count, permalink): - """Build the Slack moderation alert message.""" - mentions = " ".join(f"<@{user_id}>" for user_id in rule.alert_user_ids or []) - - return ( - f"{mentions}\n" - f":{emoji_name}: {rule.report_type} report threshold reached in <#{channel_id}>.\n" - f"Count: {reaction_count}\n" - f"{permalink}" - ).strip() diff --git a/backend/src/apps/slack/utils/reaction.py b/backend/src/apps/slack/utils/reaction.py new file mode 100644 index 0000000000..39e8057051 --- /dev/null +++ b/backend/src/apps/slack/utils/reaction.py @@ -0,0 +1,37 @@ +"""Slack reaction event helpers.""" + + +def mention_users(user_ids) -> str: + """Return Slack mention markup for the given user IDs.""" + return " ".join(f"<@{user_id}>" for user_id in user_ids or []) + + +def parse_message_reaction(event): + """Return channel, message timestamp, and emoji for a message reaction event.""" + item = event.get("item", {}) + channel_id = item.get("channel") + emoji_name = event.get("reaction") + message_ts = item.get("ts") + if ( + item.get("type") != "message" + or not channel_id + or not emoji_name + or not message_ts + or not event.get("user") + ): + return None + return channel_id, message_ts, emoji_name + + +def reaction_from_payload(payload, emoji_name: str) -> tuple[int, list[str], str] | None: + """Return count, reporter IDs, and permalink for an emoji on a reactions.get payload.""" + message = payload.get("message") or {} + permalink = message.get("permalink") or "" + for reaction in message.get("reactions") or []: + if reaction.get("name") != emoji_name: + continue + reporter_user_ids = list(reaction.get("users") or []) + count = reaction.get("count") + reaction_count = int(count) if count is not None else len(reporter_user_ids) + return reaction_count, reporter_user_ids, permalink + return None diff --git a/backend/tests/unit/apps/slack/admin/reaction_alert_test.py b/backend/tests/unit/apps/slack/admin/reaction_alert_test.py index 893283c6c0..714a9af7e7 100644 --- a/backend/tests/unit/apps/slack/admin/reaction_alert_test.py +++ b/backend/tests/unit/apps/slack/admin/reaction_alert_test.py @@ -11,4 +11,4 @@ def test_alerts_are_read_only_records(self): assert not admin.has_add_permission(request=None) assert not admin.has_delete_permission(request=None) - assert "alert_message_ts" in admin.readonly_fields + assert "reporter_user_ids" in admin.readonly_fields diff --git a/backend/tests/unit/apps/slack/events/reaction_added_test.py b/backend/tests/unit/apps/slack/events/reaction_added_test.py index 145246339e..42825eecd6 100644 --- a/backend/tests/unit/apps/slack/events/reaction_added_test.py +++ b/backend/tests/unit/apps/slack/events/reaction_added_test.py @@ -1,15 +1,234 @@ from unittest.mock import Mock +from slack_sdk.errors import SlackApiError + from apps.slack.events.reaction_added import ReactionAdded +EVENT = { + "item": {"type": "message", "channel": "C_SOURCE", "ts": "123.000"}, + "reaction": "spam", + "user": "U_REACTOR", +} + +REACTIONS_GET = { + "message": { + "permalink": "https://slack.test/message", + "reactions": [ + {"name": "spam", "count": 2, "users": ["U_REACTOR", "U_OTHER"]}, + ], + } +} + + +def slack_error(error="invalid_auth"): + """Build a Slack API error for reaction tests.""" + return SlackApiError(message="API error", response={"error": error}) + + +def mock_rule(threshold=1): + """Build a reaction rule mock.""" + return Mock( + alert_channel_id="C_ALERT", + alert_user_ids=["U_MOD"], + conversation=Mock(), + report_type="spam", + threshold=threshold, + ) + + +def mock_client(payload=None): + """Build a Slack client mock with reactions and post responses.""" + client = Mock() + client.reactions_get.return_value = payload if payload is not None else REACTIONS_GET + client.chat_postMessage.return_value = {"ts": "999.000"} + return client + + +def patch_rule_lookup(mocker, rule=None, *, missing=False): + """Patch reaction rule lookup.""" + mocker.patch( + "apps.slack.events.reaction_added.ReactionRule.for_reaction", + return_value=None if missing else (rule or mock_rule()), + ) + + +def patch_alert_lock(mocker, *, acquired=True, recorded=False): + """Patch reaction alert lookup, lock, and record helpers.""" + mocker.patch( + "apps.slack.events.reaction_added.ReactionAlert.exists_for", + return_value=recorded, + ) + acquire = mocker.patch( + "apps.slack.events.reaction_added.ReactionAlert.acquire", + return_value=acquired, + ) + release = mocker.patch("apps.slack.events.reaction_added.ReactionAlert.release") + record = mocker.patch("apps.slack.events.reaction_added.ReactionAlert.record") + return acquire, release, record + class TestReactionAdded: - def test_handle_event_delegates_to_moderation_service(self, mocker): - """Test reaction_added events delegate moderation processing to the service.""" - service = mocker.patch("apps.slack.events.reaction_added.process_reaction_added") - event = {"type": "reaction_added"} - client = Mock() + def test_handle_event_stops_when_no_rule(self, mocker): + """Test missing reaction rules skip Slack lookups and alerts.""" + client = mock_client() + patch_rule_lookup(mocker, missing=True) + acquire, _, record = patch_alert_lock(mocker) + + ReactionAdded().handle_event(EVENT, client) + + client.reactions_get.assert_not_called() + acquire.assert_not_called() + record.assert_not_called() + client.chat_postMessage.assert_not_called() + + def test_handle_event_posts_alert_and_records_it(self, mocker): + """Test threshold hit posts a Slack alert with reporters and records it.""" + client = mock_client() + rule = mock_rule(threshold=2) + patch_rule_lookup(mocker, rule) + _, release, record = patch_alert_lock(mocker) + + ReactionAdded().handle_event(EVENT, client) + + client.reactions_get.assert_called_once_with( + channel="C_SOURCE", + full=True, + timestamp="123.000", + ) + client.chat_postMessage.assert_called_once() + _, kwargs = client.chat_postMessage.call_args + assert kwargs["channel"] == "C_ALERT" + assert "<@U_MOD>" in kwargs["text"] + assert "A message in <#C_SOURCE> reached the spam report threshold." in kwargs["text"] + assert "Reported by: <@U_REACTOR> <@U_OTHER>" in kwargs["text"] + assert "https://slack.test/message" in kwargs["text"] + record.assert_called_once_with( + rule.conversation, + "123.000", + "spam", + 2, + "999.000", + reporter_user_ids=["U_REACTOR", "U_OTHER"], + ) + release.assert_called_once_with(rule.conversation, "123.000", "spam") + + def test_handle_event_skips_recorded_alert(self, mocker): + """Test an existing reaction alert skips Slack lookups and posts.""" + client = mock_client() + patch_rule_lookup(mocker) + acquire, release, record = patch_alert_lock(mocker, recorded=True) + + ReactionAdded().handle_event(EVENT, client) + + client.reactions_get.assert_not_called() + client.chat_postMessage.assert_not_called() + acquire.assert_not_called() + record.assert_not_called() + release.assert_not_called() + + def test_handle_event_skips_existing_alert(self, mocker): + """Test an in-flight reaction alert suppresses duplicate Slack posts.""" + client = mock_client() + patch_rule_lookup(mocker) + _, release, record = patch_alert_lock(mocker, acquired=False) + + ReactionAdded().handle_event(EVENT, client) + + client.reactions_get.assert_called_once() + client.chat_postMessage.assert_not_called() + record.assert_not_called() + release.assert_not_called() + + def test_handle_event_stops_below_threshold(self, mocker): + """Test reactions below the configured threshold do not create alerts.""" + client = mock_client( + { + "message": { + "permalink": "https://slack.test/message", + "reactions": [{"name": "spam", "count": 1, "users": ["U_REACTOR"]}], + } + } + ) + patch_rule_lookup(mocker, mock_rule(threshold=2)) + acquire, _, record = patch_alert_lock(mocker) + + ReactionAdded().handle_event(EVENT, client) + + acquire.assert_not_called() + record.assert_not_called() + client.chat_postMessage.assert_not_called() + + def test_handle_event_releases_lock_when_post_fails(self, mocker): + """Test a Slack post failure releases the lock and does not record an alert.""" + client = mock_client() + client.chat_postMessage.side_effect = slack_error("channel_not_found") + rule = mock_rule() + patch_rule_lookup(mocker, rule) + _, release, record = patch_alert_lock(mocker) + + ReactionAdded().handle_event(EVENT, client) + + record.assert_not_called() + release.assert_called_once_with(rule.conversation, "123.000", "spam") + + def test_handle_event_skips_when_reactions_get_fails(self, mocker): + """Test a reactions.get failure does not post or lock.""" + client = mock_client() + client.reactions_get.side_effect = slack_error("message_not_found") + patch_rule_lookup(mocker) + acquire, release, record = patch_alert_lock(mocker) + + ReactionAdded().handle_event(EVENT, client) + + acquire.assert_not_called() + record.assert_not_called() + release.assert_not_called() + client.chat_postMessage.assert_not_called() + + def test_handle_event_skips_when_emoji_missing_from_message(self, mocker): + """Test a reactions.get payload without the emoji does not post.""" + client = mock_client( + { + "message": { + "permalink": "https://slack.test/message", + "reactions": [{"name": "thumbsup", "count": 4, "users": ["U_OTHER"]}], + } + } + ) + patch_rule_lookup(mocker) + acquire, _, record = patch_alert_lock(mocker) + + ReactionAdded().handle_event(EVENT, client) + + acquire.assert_not_called() + record.assert_not_called() + client.chat_postMessage.assert_not_called() + + def test_handle_event_skips_non_message_items(self, mocker): + """Test file reactions do not look up Slack reactions.""" + client = mock_client() + patch_rule_lookup(mocker) + event = {**EVENT, "item": {"type": "file", "channel": "C_SOURCE"}} ReactionAdded().handle_event(event, client) - service.assert_called_once_with(event, client) + client.reactions_get.assert_not_called() + client.chat_postMessage.assert_not_called() + + def test_handle_event_skips_when_permalink_missing(self, mocker): + """Test a reactions.get payload without a permalink does not post.""" + client = mock_client( + { + "message": { + "reactions": [{"name": "spam", "count": 1, "users": ["U_REACTOR"]}], + } + } + ) + patch_rule_lookup(mocker) + acquire, _, record = patch_alert_lock(mocker) + + ReactionAdded().handle_event(EVENT, client) + + acquire.assert_not_called() + record.assert_not_called() + client.chat_postMessage.assert_not_called() diff --git a/backend/tests/unit/apps/slack/models/reaction_alert_test.py b/backend/tests/unit/apps/slack/models/reaction_alert_test.py new file mode 100644 index 0000000000..b266694741 --- /dev/null +++ b/backend/tests/unit/apps/slack/models/reaction_alert_test.py @@ -0,0 +1,118 @@ +from unittest.mock import Mock + +from django.db import IntegrityError + +from apps.slack.models.reaction_alert import LOCK_TTL_SECONDS, ReactionAlert + + +class TestReactionAlert: + def test_lock_key(self): + """Test lock keys include conversation, message, and report type.""" + conversation = Mock(pk=7) + + assert ( + ReactionAlert.lock_key(conversation, "123.000", "spam") + == "slack:reaction-alert:7:123.000:spam" + ) + + def test_acquire_returns_false_when_alert_exists(self, mocker): + """Test an existing alert row skips the in-flight lock.""" + conversation = Mock(pk=7) + manager = mocker.patch("apps.slack.models.reaction_alert.ReactionAlert.objects") + manager.filter.return_value.exists.return_value = True + add = mocker.patch("apps.slack.models.reaction_alert.cache.add") + + assert ReactionAlert.acquire(conversation, "123.000", "spam") is False + add.assert_not_called() + + def test_acquire_returns_false_when_lock_held(self, mocker): + """Test a held cache lock skips posting.""" + conversation = Mock(pk=7) + manager = mocker.patch("apps.slack.models.reaction_alert.ReactionAlert.objects") + manager.filter.return_value.exists.return_value = False + mocker.patch("apps.slack.models.reaction_alert.cache.add", return_value=False) + + assert ReactionAlert.acquire(conversation, "123.000", "spam") is False + + def test_acquire_returns_true_when_lock_taken(self, mocker): + """Test a free lock is acquired when no alert row exists.""" + conversation = Mock(pk=7) + manager = mocker.patch("apps.slack.models.reaction_alert.ReactionAlert.objects") + manager.filter.return_value.exists.return_value = False + add = mocker.patch("apps.slack.models.reaction_alert.cache.add", return_value=True) + + assert ReactionAlert.acquire(conversation, "123.000", "spam") is True + add.assert_called_once_with( + "slack:reaction-alert:7:123.000:spam", + 1, + timeout=LOCK_TTL_SECONDS, + ) + + def test_acquire_releases_lock_if_alert_appears_after_add(self, mocker): + """Test a row created during lock acquisition releases the lock.""" + conversation = Mock(pk=7) + manager = mocker.patch("apps.slack.models.reaction_alert.ReactionAlert.objects") + manager.filter.return_value.exists.side_effect = [False, True] + mocker.patch("apps.slack.models.reaction_alert.cache.add", return_value=True) + delete = mocker.patch("apps.slack.models.reaction_alert.cache.delete") + + assert ReactionAlert.acquire(conversation, "123.000", "spam") is False + delete.assert_called_once_with("slack:reaction-alert:7:123.000:spam") + + def test_exists_for_uses_conversation_message_and_report_type(self, mocker): + """Test exists_for looks up the unique alert row.""" + conversation = Mock() + manager = mocker.patch("apps.slack.models.reaction_alert.ReactionAlert.objects") + manager.filter.return_value.exists.return_value = True + + assert ReactionAlert.exists_for(conversation, "123.000", "spam") is True + manager.filter.assert_called_once_with( + conversation=conversation, + message_ts="123.000", + report_type="spam", + ) + + def test_record_creates_alert(self, mocker): + """Test a successful Slack post is stored as a reaction alert.""" + conversation = Mock() + manager = mocker.patch("apps.slack.models.reaction_alert.ReactionAlert.objects") + + ReactionAlert.record( + conversation, + "123.000", + "spam", + 2, + "999.000", + reporter_user_ids=["U1", "U2"], + ) + + manager.create.assert_called_once_with( + alert_message_ts="999.000", + conversation=conversation, + message_ts="123.000", + reaction_count=2, + report_type="spam", + reporter_user_ids=["U1", "U2"], + ) + + def test_record_ignores_existing_row(self, mocker): + """Test a concurrent insert does not raise.""" + manager = mocker.patch("apps.slack.models.reaction_alert.ReactionAlert.objects") + manager.create.side_effect = IntegrityError + + ReactionAlert.record( + Mock(), + "123.000", + "spam", + 1, + "999.000", + reporter_user_ids=["U1"], + ) + + def test_release_deletes_lock(self, mocker): + """Test release deletes the in-flight cache lock.""" + delete = mocker.patch("apps.slack.models.reaction_alert.cache.delete") + + ReactionAlert.release(Mock(pk=7), "123.000", "spam") + + delete.assert_called_once_with("slack:reaction-alert:7:123.000:spam") diff --git a/backend/tests/unit/apps/slack/models/reaction_rule_test.py b/backend/tests/unit/apps/slack/models/reaction_rule_test.py index bdbce7a414..1f102b5082 100644 --- a/backend/tests/unit/apps/slack/models/reaction_rule_test.py +++ b/backend/tests/unit/apps/slack/models/reaction_rule_test.py @@ -1,3 +1,5 @@ +from unittest.mock import Mock + from apps.slack.models.conversation import Conversation from apps.slack.models.reaction_rule import ReactionRule from apps.slack.models.workspace import Workspace @@ -11,3 +13,31 @@ def test_str(self): rule = ReactionRule(conversation=conversation, emoji_name="spam") assert str(rule) == "OWASP #general :spam" + + def test_report_type_defaults_to_spam(self): + """Test report type is limited to spam.""" + assert ReactionRule.ReportType.SPAM == "spam" + assert ReactionRule.ReportType.choices == [("spam", "Spam")] + + def test_for_reaction_returns_matching_rule(self, mocker): + """Test reaction rule lookup returns the matching active rule.""" + rule = Mock() + manager = mocker.patch("apps.slack.models.reaction_rule.ReactionRule.objects") + manager.select_related.return_value.filter.return_value.first.return_value = rule + + result = ReactionRule.for_reaction("C123", "spam") + + assert result is rule + manager.select_related.assert_called_once_with("conversation") + manager.select_related.return_value.filter.assert_called_once_with( + conversation__slack_channel_id="C123", + emoji_name="spam", + is_active=True, + ) + + def test_for_reaction_returns_none_when_missing(self, mocker): + """Test reaction rule lookup returns None when no rule exists.""" + manager = mocker.patch("apps.slack.models.reaction_rule.ReactionRule.objects") + manager.select_related.return_value.filter.return_value.first.return_value = None + + assert ReactionRule.for_reaction("C123", "spam") is None diff --git a/backend/tests/unit/apps/slack/services/reaction_test.py b/backend/tests/unit/apps/slack/services/reaction_test.py deleted file mode 100644 index c37f72f704..0000000000 --- a/backend/tests/unit/apps/slack/services/reaction_test.py +++ /dev/null @@ -1,191 +0,0 @@ -from unittest.mock import Mock - -from django.db import IntegrityError -from slack_sdk.errors import SlackApiError - -from apps.slack.models.reaction_alert import ReactionAlert -from apps.slack.services.reaction import get_or_create_alert, process_reaction_added - -EVENT = { - "item": {"type": "message", "channel": "C_SOURCE", "ts": "123.000"}, - "reaction": "spam", -} - - -def slack_error(error="invalid_auth"): - """Build a Slack API error for service tests.""" - return SlackApiError(message="API error", response={"error": error}) - - -def mock_rule(threshold=1): - """Build a reaction rule mock.""" - return Mock( - alert_channel_id="C_ALERT", - alert_user_ids=["U_MOD"], - report_type="spam", - threshold=threshold, - ) - - -def mock_client(users=None): - """Build a Slack client mock with reaction and post responses.""" - client = Mock() - client.reactions_get.return_value = { - "message": { - "reactions": [ - { - "count": len(set(users or ["U1"])), - "name": "spam", - "users": users or ["U1"], - } - ] - } - } - client.chat_getPermalink.return_value = {"permalink": "https://slack.test/message"} - client.chat_postMessage.return_value = {"ts": "999.000"} - return client - - -def patch_rule_lookup(mocker, rule=None): - """Patch conversation and reaction rule lookups.""" - mocker.patch( - "apps.slack.services.reaction.Conversation.objects.get", - return_value=Mock(), - ) - mocker.patch( - "apps.slack.services.reaction.ReactionRule.objects.get", - return_value=rule or mock_rule(), - ) - - -class TestReactionService: - def test_process_reaction_added_posts_alert_and_records_it(self, mocker): - """Test threshold hit posts a Slack alert and records its message timestamp.""" - client = mock_client(users=["U1", "U2", "U1"]) - reaction_alert = Mock() - patch_rule_lookup(mocker, mock_rule(threshold=2)) - mocker.patch( - "apps.slack.services.reaction.ReactionAlert.objects.get_or_create", - return_value=(reaction_alert, True), - ) - - process_reaction_added(EVENT, client) - - client.reactions_get.assert_called_once_with(channel="C_SOURCE", timestamp="123.000") - client.chat_getPermalink.assert_called_once_with(channel="C_SOURCE", message_ts="123.000") - client.chat_postMessage.assert_called_once() - _, kwargs = client.chat_postMessage.call_args - assert kwargs["channel"] == "C_ALERT" - assert "<@U_MOD>" in kwargs["text"] - assert "spam report threshold reached" in kwargs["text"] - assert "Count: 2" in kwargs["text"] - assert "https://slack.test/message" in kwargs["text"] - assert reaction_alert.alert_message_ts == "999.000" - reaction_alert.save.assert_called_once_with(update_fields=["alert_message_ts"]) - - def test_process_reaction_added_skips_existing_alert(self, mocker): - """Test an existing reaction alert suppresses duplicate Slack posts.""" - client = mock_client() - reaction_alert = Mock() - patch_rule_lookup(mocker) - mocker.patch( - "apps.slack.services.reaction.ReactionAlert.objects.get_or_create", - return_value=(reaction_alert, False), - ) - - process_reaction_added(EVENT, client) - - client.chat_getPermalink.assert_not_called() - client.chat_postMessage.assert_not_called() - reaction_alert.save.assert_not_called() - - def test_process_reaction_added_stops_below_threshold(self, mocker): - """Test reactions below the configured threshold do not create alerts.""" - client = mock_client() - patch_rule_lookup(mocker, mock_rule(threshold=2)) - get_or_create = mocker.patch( - "apps.slack.services.reaction.ReactionAlert.objects.get_or_create" - ) - - process_reaction_added(EVENT, client) - - get_or_create.assert_not_called() - client.chat_postMessage.assert_not_called() - - def test_process_reaction_added_deletes_claimed_alert_when_post_fails(self, mocker): - """Test a claimed alert is deleted when Slack posting fails.""" - client = mock_client() - client.chat_postMessage.side_effect = slack_error("channel_not_found") - reaction_alert = Mock() - patch_rule_lookup(mocker) - mocker.patch( - "apps.slack.services.reaction.ReactionAlert.objects.get_or_create", - return_value=(reaction_alert, True), - ) - - process_reaction_added(EVENT, client) - - reaction_alert.delete.assert_called_once() - reaction_alert.save.assert_not_called() - - def test_process_reaction_added_deletes_claimed_alert_when_permalink_fails(self, mocker): - """Test a claimed alert is deleted when permalink lookup fails.""" - client = mock_client() - client.chat_getPermalink.side_effect = slack_error("message_not_found") - reaction_alert = Mock() - patch_rule_lookup(mocker) - mocker.patch( - "apps.slack.services.reaction.ReactionAlert.objects.get_or_create", - return_value=(reaction_alert, True), - ) - - process_reaction_added(EVENT, client) - - reaction_alert.delete.assert_called_once() - client.chat_postMessage.assert_not_called() - reaction_alert.save.assert_not_called() - - def test_process_reaction_added_stops_when_reactions_get_fails(self, mocker): - """Test Slack reaction API failures stop before claiming an alert.""" - client = Mock() - client.reactions_get.side_effect = slack_error() - patch_rule_lookup(mocker) - get_or_create = mocker.patch( - "apps.slack.services.reaction.ReactionAlert.objects.get_or_create" - ) - - process_reaction_added(EVENT, client) - - get_or_create.assert_not_called() - client.chat_postMessage.assert_not_called() - - def test_process_reaction_added_uses_slack_reaction_count(self, mocker): - """Test Slack reaction count is used when the users list is truncated.""" - client = mock_client(users=["U1"]) - client.reactions_get.return_value = { - "message": {"reactions": [{"count": 3, "name": "spam", "users": ["U1"]}]} - } - reaction_alert = Mock() - patch_rule_lookup(mocker, mock_rule(threshold=3)) - mocker.patch( - "apps.slack.services.reaction.ReactionAlert.objects.get_or_create", - return_value=(reaction_alert, True), - ) - - process_reaction_added(EVENT, client) - - _, kwargs = client.chat_postMessage.call_args - assert "Count: 3" in kwargs["text"] - - def test_get_or_create_alert_handles_deleted_race_fallback(self, mocker): - """Test IntegrityError fallback tolerates a concurrently deleted alert row.""" - manager = mocker.patch("apps.slack.services.reaction.ReactionAlert.objects") - manager.get_or_create.side_effect = IntegrityError - manager.get.side_effect = ReactionAlert.DoesNotExist - - alert, created = get_or_create_alert(Mock(), "123.000", "spam", 1) - - assert alert is None - assert not created - assert manager.get_or_create.call_count == 2 - assert manager.get.call_count == 2 diff --git a/backend/tests/unit/apps/slack/utils/__init__.py b/backend/tests/unit/apps/slack/utils/__init__.py new file mode 100644 index 0000000000..cc54cc7df2 --- /dev/null +++ b/backend/tests/unit/apps/slack/utils/__init__.py @@ -0,0 +1 @@ +"""Slack utils unit tests.""" diff --git a/backend/tests/unit/apps/slack/utils/reaction_test.py b/backend/tests/unit/apps/slack/utils/reaction_test.py new file mode 100644 index 0000000000..b64137b474 --- /dev/null +++ b/backend/tests/unit/apps/slack/utils/reaction_test.py @@ -0,0 +1,60 @@ +from apps.slack.utils.reaction import mention_users, parse_message_reaction, reaction_from_payload + +EVENT = { + "item": {"type": "message", "channel": "C_SOURCE", "ts": "123.000"}, + "reaction": "spam", + "user": "U_REACTOR", +} + +PAYLOAD = { + "message": { + "permalink": "https://slack.test/message", + "reactions": [ + {"name": "thumbsup", "count": 1, "users": ["U_OTHER"]}, + {"name": "spam", "count": 2, "users": ["U_REACTOR", "U_OTHER"]}, + ], + } +} + + +class TestParseMessageReaction: + def test_parse_message_reaction_returns_details(self): + """Test message reaction events expose channel, timestamp, and emoji.""" + assert parse_message_reaction(EVENT) == ("C_SOURCE", "123.000", "spam") + + def test_parse_message_reaction_skips_non_message_items(self): + """Test file reactions are ignored.""" + event = {**EVENT, "item": {"type": "file", "channel": "C_SOURCE"}} + + assert parse_message_reaction(event) is None + + def test_parse_message_reaction_skips_missing_user(self): + """Test reaction events without a user are ignored.""" + event = {**EVENT, "user": ""} + + assert parse_message_reaction(event) is None + + +class TestReactionFromPayload: + def test_reaction_from_payload_returns_matching_emoji(self): + """Test reactions.get payloads expose count, reporters, and permalink.""" + assert reaction_from_payload(PAYLOAD, "spam") == ( + 2, + ["U_REACTOR", "U_OTHER"], + "https://slack.test/message", + ) + + def test_reaction_from_payload_returns_none_when_emoji_missing(self): + """Test an unmatched emoji does not produce a snapshot.""" + assert reaction_from_payload(PAYLOAD, "flag") is None + + +class TestMentionUsers: + def test_mention_users_joins_ids(self): + """Test Slack user IDs are formatted as mentions.""" + assert mention_users(["U1", "U2"]) == "<@U1> <@U2>" + + def test_mention_users_handles_empty(self): + """Test missing user IDs produce no mention markup.""" + assert mention_users([]) == "" + assert mention_users(None) == "" From 2500f933853cc5e52da14c7e46ed6b61602c30f5 Mon Sep 17 00:00:00 2001 From: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:07:40 -0700 Subject: [PATCH 09/10] Update code Signed-off-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> --- backend/src/apps/slack/admin/reaction_rule.py | 4 +- .../src/apps/slack/events/reaction_added.py | 112 ++++++++++++------ .../migrations/0028_reactionrule_emojis.py | 39 ++++++ .../0029_alter_reactionrule_threshold.py | 19 +++ .../0030_alter_reaction_help_text.py | 30 +++++ .../src/apps/slack/models/reaction_alert.py | 55 ++++++--- .../src/apps/slack/models/reaction_rule.py | 87 ++++++++++---- backend/src/apps/slack/utils/reaction.py | 41 +++++-- .../apps/slack/events/reaction_added_test.py | 111 +++++++++++++++-- .../apps/slack/models/reaction_alert_test.py | 95 +++++++++++---- .../apps/slack/models/reaction_rule_test.py | 72 +++++++++-- .../unit/apps/slack/utils/reaction_test.py | 67 ++++++++++- 12 files changed, 592 insertions(+), 140 deletions(-) create mode 100644 backend/src/apps/slack/migrations/0028_reactionrule_emojis.py create mode 100644 backend/src/apps/slack/migrations/0029_alter_reactionrule_threshold.py create mode 100644 backend/src/apps/slack/migrations/0030_alter_reaction_help_text.py diff --git a/backend/src/apps/slack/admin/reaction_rule.py b/backend/src/apps/slack/admin/reaction_rule.py index ce5251602f..bdc5e9b2b1 100644 --- a/backend/src/apps/slack/admin/reaction_rule.py +++ b/backend/src/apps/slack/admin/reaction_rule.py @@ -12,7 +12,7 @@ class ReactionRuleAdmin(admin.ModelAdmin): autocomplete_fields = ("conversation",) list_display = ( "conversation", - "emoji_name", + "emojis", "report_type", "threshold", "is_active", @@ -23,6 +23,6 @@ class ReactionRuleAdmin(admin.ModelAdmin): ) search_fields = ( "conversation__name", - "emoji_name", + "emojis", "alert_channel_id", ) diff --git a/backend/src/apps/slack/events/reaction_added.py b/backend/src/apps/slack/events/reaction_added.py index 31f99ac85a..90164e3213 100644 --- a/backend/src/apps/slack/events/reaction_added.py +++ b/backend/src/apps/slack/events/reaction_added.py @@ -8,13 +8,36 @@ from apps.slack.events.event import EventBase from apps.slack.models.reaction_alert import ReactionAlert from apps.slack.models.reaction_rule import ReactionRule -from apps.slack.utils.reaction import mention_users, parse_message_reaction, reaction_from_payload +from apps.slack.utils.reaction import ( + format_emojis, + mention_users, + parse_message_reaction, + reaction_from_payload, +) logger = logging.getLogger(__name__) -def fetch_reaction(client, channel_id: str, message_ts: str, emoji_name: str): - """Return Slack's current reaction snapshot for the emoji, or None.""" +def fetch_permalink(client, channel_id: str, message_ts: str) -> str: + """Return a Slack permalink for the message, or an empty string.""" + try: + return ( + client.chat_getPermalink( + channel=channel_id, + message_ts=message_ts, + ).get("permalink") + or "" + ) + except SlackApiError as e: + logger.warning( + "Could not fetch Slack permalink for moderation alert: %s", + e.response.get("error", "unknown_error"), + ) + return "" + + +def fetch_reaction(client, channel_id: str, message_ts: str, emojis: list[str]): + """Return Slack's current unique-reporter snapshot for the rule emojis, or None.""" try: payload = client.reactions_get( channel=channel_id, @@ -27,7 +50,8 @@ def fetch_reaction(client, channel_id: str, message_ts: str, emoji_name: str): e.response.get("error", "unknown_error"), ) return None - return reaction_from_payload(payload, emoji_name) + + return reaction_from_payload(payload, emojis) class ReactionAdded(EventBase): @@ -41,53 +65,63 @@ def handle_event(self, event, client): return channel_id, message_ts, emoji_name = details - if (rule := ReactionRule.for_reaction(channel_id, emoji_name)) is None: + if (rule := ReactionRule.for_emoji(channel_id, emoji_name)) is None: return + if ReactionAlert.exists_for(rule.conversation, message_ts, rule.report_type): return - if (snapshot := fetch_reaction(client, channel_id, message_ts, emoji_name)) is None: + if (snapshot := fetch_reaction(client, channel_id, message_ts, rule.emojis)) is None: return reaction_count, reporter_user_ids, permalink = snapshot - if reaction_count < rule.threshold or not permalink: + if reaction_count < rule.threshold: return # Lock in-flight posts; the DB row is written only after Slack succeeds. - if not ReactionAlert.acquire(rule.conversation, message_ts, rule.report_type): + if ( + owner := ReactionAlert.acquire(rule.conversation, message_ts, rule.report_type) + ) is None: return try: - alert_users = mention_users(rule.alert_user_ids) - reporters = mention_users(reporter_user_ids) - text = ( - f"{alert_users}\n" - f":{emoji_name}: A message in <#{channel_id}> reached the " - f"{rule.report_type} report threshold." - ) - if reporters: - text = f"{text}\nReported by: {reporters}" - text = f"{text}\n{permalink}".strip() - - try: - alert = client.chat_postMessage( - blocks=[markdown(text)], - channel=rule.alert_channel_id, - text=text, - ) - except SlackApiError as e: - logger.warning( - "Could not post Slack moderation alert: %s", - e.response.get("error", "unknown_error"), - ) - else: - ReactionAlert.record( - rule.conversation, - message_ts, - rule.report_type, - reaction_count, - alert.get("ts", ""), - reporter_user_ids=reporter_user_ids, + if not permalink: + permalink = fetch_permalink(client, channel_id, message_ts) + + if ReactionAlert.renew(rule.conversation, message_ts, rule.report_type, owner): + alert_users = mention_users(rule.alert_user_ids) + reporters = mention_users(reporter_user_ids) + emojis = format_emojis(rule.emojis) + text = ( + f"{alert_users}\n" + f"A message in <#{channel_id}> reached the " + f"{rule.report_type} report threshold." ) + if reporters: + text = f"{text}\nReported by: {reporters} using the following emojis: {emojis}" + if permalink: + text = f"{text}\n{permalink}" + text = text.strip() + + try: + alert = client.chat_postMessage( + blocks=[markdown(text)], + channel=rule.alert_channel_id, + text=text, + ) + except SlackApiError as e: + logger.warning( + "Could not post Slack moderation alert: %s", + e.response.get("error", "unknown_error"), + ) + else: + ReactionAlert.record( + rule.conversation, + message_ts, + rule.report_type, + reaction_count, + alert.get("ts", ""), + reporter_user_ids=reporter_user_ids, + ) finally: - ReactionAlert.release(rule.conversation, message_ts, rule.report_type) + ReactionAlert.release(rule.conversation, message_ts, rule.report_type, owner) diff --git a/backend/src/apps/slack/migrations/0028_reactionrule_emojis.py b/backend/src/apps/slack/migrations/0028_reactionrule_emojis.py new file mode 100644 index 0000000000..66b4933487 --- /dev/null +++ b/backend/src/apps/slack/migrations/0028_reactionrule_emojis.py @@ -0,0 +1,39 @@ +from django.db import migrations, models + + +def copy_emoji_name_to_emojis(apps, _schema_editor): + """Copy each reaction rule's single emoji name into the emojis list.""" + reaction_rule_model = apps.get_model("slack", "ReactionRule") + for rule in reaction_rule_model.objects.all(): + rule.emojis = [rule.emoji_name] if rule.emoji_name else [] + rule.save(update_fields=["emojis"]) + + +class Migration(migrations.Migration): + dependencies = [ + ("slack", "0027_alter_reactionrule_emoji_name"), + ] + + operations = [ + migrations.AddField( + model_name="reactionrule", + name="emojis", + field=models.JSONField( + blank=True, + default=list, + help_text="Slack emojis that trigger this reaction rule.", + ), + ), + migrations.RunPython( + copy_emoji_name_to_emojis, + migrations.RunPython.noop, + ), + migrations.AlterUniqueTogether( + name="reactionrule", + unique_together={("conversation", "report_type")}, + ), + migrations.RemoveField( + model_name="reactionrule", + name="emoji_name", + ), + ] diff --git a/backend/src/apps/slack/migrations/0029_alter_reactionrule_threshold.py b/backend/src/apps/slack/migrations/0029_alter_reactionrule_threshold.py new file mode 100644 index 0000000000..832294b91f --- /dev/null +++ b/backend/src/apps/slack/migrations/0029_alter_reactionrule_threshold.py @@ -0,0 +1,19 @@ +import django.core.validators +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("slack", "0028_reactionrule_emojis"), + ] + + operations = [ + migrations.AlterField( + model_name="reactionrule", + name="threshold", + field=models.PositiveSmallIntegerField( + default=10, + validators=[django.core.validators.MinValueValidator(1)], + ), + ), + ] diff --git a/backend/src/apps/slack/migrations/0030_alter_reaction_help_text.py b/backend/src/apps/slack/migrations/0030_alter_reaction_help_text.py new file mode 100644 index 0000000000..d212027284 --- /dev/null +++ b/backend/src/apps/slack/migrations/0030_alter_reaction_help_text.py @@ -0,0 +1,30 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("slack", "0029_alter_reactionrule_threshold"), + ] + + operations = [ + migrations.AlterField( + model_name="reactionrule", + name="alert_user_ids", + field=models.JSONField( + blank=True, + default=list, + help_text="Slack user IDs mentioned when this reaction rule triggers.", + ), + ), + migrations.AlterField( + model_name="reactionalert", + name="reporter_user_ids", + field=models.JSONField( + blank=True, + default=list, + help_text=( + "Slack user IDs that reacted with a listed emoji when the alert was posted." + ), + ), + ), + ] diff --git a/backend/src/apps/slack/models/reaction_alert.py b/backend/src/apps/slack/models/reaction_alert.py index 62c3436863..3c64a82f21 100644 --- a/backend/src/apps/slack/models/reaction_alert.py +++ b/backend/src/apps/slack/models/reaction_alert.py @@ -1,12 +1,14 @@ """Store emitted Slack reaction alerts.""" +from uuid import uuid4 + from django.core.cache import cache from django.db import IntegrityError, models from apps.common.models import TimestampedModel from apps.slack.models.conversation import Conversation -LOCK_TTL_SECONDS = 30 +LOCK_TTL_SECONDS = 120 class ReactionAlert(TimestampedModel): @@ -36,27 +38,28 @@ class Meta: reporter_user_ids = models.JSONField( blank=True, default=list, - help_text="Slack user IDs that had the triggering reaction when the alert was posted.", + help_text="Slack user IDs that reacted with a listed emoji when the alert was posted.", ) # FKs. conversation = models.ForeignKey(Conversation, on_delete=models.CASCADE) @staticmethod - def acquire(conversation: Conversation, message_ts: str, report_type: str) -> bool: - """Return True if this process should post the alert.""" + def acquire(conversation: Conversation, message_ts: str, report_type: str) -> str | None: + """Return a lock owner if this process should post the alert.""" if ReactionAlert.exists_for(conversation, message_ts, report_type): - return False + return None key = ReactionAlert.lock_key(conversation, message_ts, report_type) - if not cache.add(key, 1, timeout=LOCK_TTL_SECONDS): - return False + owner = uuid4().hex + if not cache.add(key, owner, timeout=LOCK_TTL_SECONDS): + return None if ReactionAlert.exists_for(conversation, message_ts, report_type): - cache.delete(key) - return False + ReactionAlert.release(conversation, message_ts, report_type, owner) + return None - return True + return owner @staticmethod def exists_for(conversation: Conversation, message_ts: str, report_type: str) -> bool: @@ -82,7 +85,7 @@ def record( *, reporter_user_ids: list[str], ) -> None: - """Store that an alert was sent, ignoring a concurrent insert.""" + """Store that an alert was sent, ignoring a concurrent unique insert.""" try: ReactionAlert.objects.create( alert_message_ts=alert_message_ts, @@ -93,9 +96,31 @@ def record( reporter_user_ids=reporter_user_ids, ) except IntegrityError: - return + if ReactionAlert.exists_for(conversation, message_ts, report_type): + return + raise + + @staticmethod + def release( + conversation: Conversation, + message_ts: str, + report_type: str, + owner: str, + ) -> None: + """Release the in-flight lock if this process still owns it.""" + key = ReactionAlert.lock_key(conversation, message_ts, report_type) + if cache.get(key) == owner: + cache.delete(key) @staticmethod - def release(conversation: Conversation, message_ts: str, report_type: str) -> None: - """Release the in-flight lock so a later reaction can retry.""" - cache.delete(ReactionAlert.lock_key(conversation, message_ts, report_type)) + def renew( + conversation: Conversation, + message_ts: str, + report_type: str, + owner: str, + ) -> bool: + """Extend the lock TTL if this process still owns it.""" + key = ReactionAlert.lock_key(conversation, message_ts, report_type) + if cache.get(key) != owner: + return False + return bool(cache.touch(key, LOCK_TTL_SECONDS)) diff --git a/backend/src/apps/slack/models/reaction_rule.py b/backend/src/apps/slack/models/reaction_rule.py index eceafbd282..a84a8a864b 100644 --- a/backend/src/apps/slack/models/reaction_rule.py +++ b/backend/src/apps/slack/models/reaction_rule.py @@ -1,10 +1,12 @@ """Store channel-specific Slack reaction rules.""" +from django.core.exceptions import ValidationError from django.core.validators import MinValueValidator from django.db import models from apps.common.models import TimestampedModel from apps.slack.models.conversation import Conversation +from apps.slack.utils.reaction import format_emojis class ReactionRule(TimestampedModel): @@ -19,19 +21,21 @@ class Meta: """Model options.""" db_table = "slack_reaction_rules" - unique_together = ("conversation", "emoji_name") + unique_together = ("conversation", "report_type") alert_channel_id = models.CharField( max_length=50, help_text="Slack channel ID where reaction alerts are posted.", ) - alert_user_ids = models.JSONField(blank=True, default=list) - emoji_name = models.CharField( - max_length=64, - help_text=( - "Slack emoji name that triggers this reaction rule, without leading or " - "trailing colons (spam, not :spam:)." - ), + alert_user_ids = models.JSONField( + blank=True, + default=list, + help_text="Slack user IDs mentioned when this reaction rule triggers.", + ) + emojis = models.JSONField( + blank=True, + default=list, + help_text="Slack emojis that trigger this reaction rule.", ) is_active = models.BooleanField(default=True) report_type = models.CharField( @@ -40,24 +44,67 @@ class Meta: default=ReportType.SPAM, help_text="Report category recorded when this reaction rule triggers.", ) - threshold = models.PositiveSmallIntegerField(default=3, validators=[MinValueValidator(1)]) + threshold = models.PositiveSmallIntegerField(default=10, validators=[MinValueValidator(1)]) # FKs. conversation = models.ForeignKey(Conversation, on_delete=models.CASCADE) def __str__(self): """Human readable representation.""" - return f"{self.conversation} :{self.emoji_name}" + return f"{self.conversation} {format_emojis(self.emojis)}".strip() + + def clean(self): + """Validate emojis and reject overlap with other active rules on the channel.""" + super().clean() + names = self.emojis + if not isinstance(names, list) or not names: + raise ValidationError({"emojis": "Enter at least one Slack emoji."}) + + cleaned = [] + seen: set[str] = set() + for raw_name in names: + if not isinstance(raw_name, str) or not raw_name.strip(): + raise ValidationError({"emojis": "Emojis must be non-empty strings."}) + name = raw_name.strip() + if name.startswith(":") or name.endswith(":"): + raise ValidationError( + {"emojis": "Enter emojis without colons (spam, not :spam:)."} + ) + if name in seen: + raise ValidationError({"emojis": f"Duplicate emoji: {name}."}) + seen.add(name) + cleaned.append(name) + self.emojis = cleaned + + if not self.conversation_id: + return + + others = ReactionRule.objects.filter( + conversation_id=self.conversation_id, + is_active=True, + ) + if self.pk: + others = others.exclude(pk=self.pk) + overlap = seen.intersection( + name for other in others if isinstance(other.emojis, list) for name in other.emojis + ) + if overlap: + raise ValidationError( + { + "emojis": ( + "These emojis are already used by another rule on this " + f"channel: {', '.join(sorted(overlap))}." + ) + } + ) @staticmethod - def for_reaction(channel_id: str, emoji_name: str) -> "ReactionRule | None": + def for_emoji(channel_id: str, emoji_name: str) -> "ReactionRule | None": """Get the active reaction rule for a channel and emoji, if configured.""" - return ( - ReactionRule.objects.select_related("conversation") - .filter( - conversation__slack_channel_id=channel_id, - emoji_name=emoji_name, - is_active=True, - ) - .first() - ) + for rule in ReactionRule.objects.select_related("conversation").filter( + conversation__slack_channel_id=channel_id, + is_active=True, + ): + if isinstance(rule.emojis, list) and emoji_name in rule.emojis: + return rule + return None diff --git a/backend/src/apps/slack/utils/reaction.py b/backend/src/apps/slack/utils/reaction.py index 39e8057051..d7652f5745 100644 --- a/backend/src/apps/slack/utils/reaction.py +++ b/backend/src/apps/slack/utils/reaction.py @@ -1,9 +1,18 @@ """Slack reaction event helpers.""" -def mention_users(user_ids) -> str: +def format_emojis(emojis: object) -> str: + """Return Slack emoji markup for the given emoji names.""" + if not emojis or not isinstance(emojis, list): + return "" + return " ".join(f":{name}:" for name in emojis if name) + + +def mention_users(user_ids: object) -> str: """Return Slack mention markup for the given user IDs.""" - return " ".join(f"<@{user_id}>" for user_id in user_ids or []) + if not user_ids or not isinstance(user_ids, list): + return "" + return " ".join(f"<@{user_id}>" for user_id in user_ids if user_id) def parse_message_reaction(event): @@ -23,15 +32,27 @@ def parse_message_reaction(event): return channel_id, message_ts, emoji_name -def reaction_from_payload(payload, emoji_name: str) -> tuple[int, list[str], str] | None: - """Return count, reporter IDs, and permalink for an emoji on a reactions.get payload.""" +def reaction_from_payload(payload, emojis: object) -> tuple[int, list[str], str] | None: + """Return unique reporter IDs and permalink for listed emojis on a reactions.get payload.""" + if not isinstance(emojis, list): + return None + wanted = {name for name in emojis if name} + if not wanted: + return None + message = payload.get("message") or {} permalink = message.get("permalink") or "" + reporters: list[str] = [] + seen: set[str] = set() + matched = False for reaction in message.get("reactions") or []: - if reaction.get("name") != emoji_name: + if reaction.get("name") not in wanted: continue - reporter_user_ids = list(reaction.get("users") or []) - count = reaction.get("count") - reaction_count = int(count) if count is not None else len(reporter_user_ids) - return reaction_count, reporter_user_ids, permalink - return None + matched = True + for user_id in reaction.get("users") or []: + if user_id and user_id not in seen: + seen.add(user_id) + reporters.append(user_id) + if not matched: + return None + return len(reporters), reporters, permalink diff --git a/backend/tests/unit/apps/slack/events/reaction_added_test.py b/backend/tests/unit/apps/slack/events/reaction_added_test.py index 42825eecd6..3099e1021f 100644 --- a/backend/tests/unit/apps/slack/events/reaction_added_test.py +++ b/backend/tests/unit/apps/slack/events/reaction_added_test.py @@ -10,6 +10,8 @@ "user": "U_REACTOR", } +LOCK_OWNER = "lock-owner" + REACTIONS_GET = { "message": { "permalink": "https://slack.test/message", @@ -25,12 +27,13 @@ def slack_error(error="invalid_auth"): return SlackApiError(message="API error", response={"error": error}) -def mock_rule(threshold=1): +def mock_rule(threshold=1, emojis=None): """Build a reaction rule mock.""" return Mock( alert_channel_id="C_ALERT", alert_user_ids=["U_MOD"], conversation=Mock(), + emojis=emojis or ["spam"], report_type="spam", threshold=threshold, ) @@ -47,12 +50,12 @@ def mock_client(payload=None): def patch_rule_lookup(mocker, rule=None, *, missing=False): """Patch reaction rule lookup.""" mocker.patch( - "apps.slack.events.reaction_added.ReactionRule.for_reaction", + "apps.slack.events.reaction_added.ReactionRule.for_emoji", return_value=None if missing else (rule or mock_rule()), ) -def patch_alert_lock(mocker, *, acquired=True, recorded=False): +def patch_alert_lock(mocker, *, acquired=True, recorded=False, renewed=True): """Patch reaction alert lookup, lock, and record helpers.""" mocker.patch( "apps.slack.events.reaction_added.ReactionAlert.exists_for", @@ -60,7 +63,11 @@ def patch_alert_lock(mocker, *, acquired=True, recorded=False): ) acquire = mocker.patch( "apps.slack.events.reaction_added.ReactionAlert.acquire", - return_value=acquired, + return_value=LOCK_OWNER if acquired else None, + ) + mocker.patch( + "apps.slack.events.reaction_added.ReactionAlert.renew", + return_value=renewed, ) release = mocker.patch("apps.slack.events.reaction_added.ReactionAlert.release") record = mocker.patch("apps.slack.events.reaction_added.ReactionAlert.record") @@ -96,11 +103,15 @@ def test_handle_event_posts_alert_and_records_it(self, mocker): timestamp="123.000", ) client.chat_postMessage.assert_called_once() + client.chat_getPermalink.assert_not_called() _, kwargs = client.chat_postMessage.call_args assert kwargs["channel"] == "C_ALERT" assert "<@U_MOD>" in kwargs["text"] assert "A message in <#C_SOURCE> reached the spam report threshold." in kwargs["text"] - assert "Reported by: <@U_REACTOR> <@U_OTHER>" in kwargs["text"] + assert ( + "Reported by: <@U_REACTOR> <@U_OTHER> using the following emojis: :spam:" + in kwargs["text"] + ) assert "https://slack.test/message" in kwargs["text"] record.assert_called_once_with( rule.conversation, @@ -110,7 +121,7 @@ def test_handle_event_posts_alert_and_records_it(self, mocker): "999.000", reporter_user_ids=["U_REACTOR", "U_OTHER"], ) - release.assert_called_once_with(rule.conversation, "123.000", "spam") + release.assert_called_once_with(rule.conversation, "123.000", "spam", LOCK_OWNER) def test_handle_event_skips_recorded_alert(self, mocker): """Test an existing reaction alert skips Slack lookups and posts.""" @@ -169,7 +180,20 @@ def test_handle_event_releases_lock_when_post_fails(self, mocker): ReactionAdded().handle_event(EVENT, client) record.assert_not_called() - release.assert_called_once_with(rule.conversation, "123.000", "spam") + release.assert_called_once_with(rule.conversation, "123.000", "spam", LOCK_OWNER) + + def test_handle_event_skips_post_when_lock_lease_is_lost(self, mocker): + """Test a lost in-flight lock skips posting and still releases the owned lock.""" + client = mock_client() + rule = mock_rule() + patch_rule_lookup(mocker, rule) + _, release, record = patch_alert_lock(mocker, renewed=False) + + ReactionAdded().handle_event(EVENT, client) + + client.chat_postMessage.assert_not_called() + record.assert_not_called() + release.assert_called_once_with(rule.conversation, "123.000", "spam", LOCK_OWNER) def test_handle_event_skips_when_reactions_get_fails(self, mocker): """Test a reactions.get failure does not post or lock.""" @@ -204,6 +228,40 @@ def test_handle_event_skips_when_emoji_missing_from_message(self, mocker): record.assert_not_called() client.chat_postMessage.assert_not_called() + def test_handle_event_counts_unique_reporters_across_emojis(self, mocker): + """Test a shared emoji list counts each Slack user once toward the threshold.""" + client = mock_client( + { + "message": { + "permalink": "https://slack.test/message", + "reactions": [ + {"name": "spam", "count": 2, "users": ["U1", "U2"]}, + {"name": "flag", "count": 2, "users": ["U2", "U3"]}, + ], + } + } + ) + rule = mock_rule(threshold=3, emojis=["spam", "flag"]) + patch_rule_lookup(mocker, rule) + _, _, record = patch_alert_lock(mocker) + + ReactionAdded().handle_event(EVENT, client) + + client.chat_postMessage.assert_called_once() + _, kwargs = client.chat_postMessage.call_args + assert ( + "Reported by: <@U1> <@U2> <@U3> using the following emojis: :spam: :flag:" + in kwargs["text"] + ) + record.assert_called_once_with( + rule.conversation, + "123.000", + "spam", + 3, + "999.000", + reporter_user_ids=["U1", "U2", "U3"], + ) + def test_handle_event_skips_non_message_items(self, mocker): """Test file reactions do not look up Slack reactions.""" client = mock_client() @@ -215,8 +273,8 @@ def test_handle_event_skips_non_message_items(self, mocker): client.reactions_get.assert_not_called() client.chat_postMessage.assert_not_called() - def test_handle_event_skips_when_permalink_missing(self, mocker): - """Test a reactions.get payload without a permalink does not post.""" + def test_handle_event_fetches_permalink_when_missing(self, mocker): + """Test a missing reactions.get permalink is fetched before posting.""" client = mock_client( { "message": { @@ -224,11 +282,38 @@ def test_handle_event_skips_when_permalink_missing(self, mocker): } } ) + client.chat_getPermalink.return_value = {"permalink": "https://slack.test/fallback"} patch_rule_lookup(mocker) - acquire, _, record = patch_alert_lock(mocker) + _, _, record = patch_alert_lock(mocker) ReactionAdded().handle_event(EVENT, client) - acquire.assert_not_called() - record.assert_not_called() - client.chat_postMessage.assert_not_called() + client.chat_getPermalink.assert_called_once_with( + channel="C_SOURCE", + message_ts="123.000", + ) + client.chat_postMessage.assert_called_once() + _, kwargs = client.chat_postMessage.call_args + assert "https://slack.test/fallback" in kwargs["text"] + record.assert_called_once() + + def test_handle_event_posts_when_permalink_unavailable(self, mocker): + """Test a threshold hit is still posted when no permalink can be fetched.""" + client = mock_client( + { + "message": { + "reactions": [{"name": "spam", "count": 1, "users": ["U_REACTOR"]}], + } + } + ) + client.chat_getPermalink.side_effect = slack_error("message_not_found") + patch_rule_lookup(mocker) + _, release, record = patch_alert_lock(mocker) + + ReactionAdded().handle_event(EVENT, client) + + client.chat_postMessage.assert_called_once() + _, kwargs = client.chat_postMessage.call_args + assert "https://" not in kwargs["text"] + record.assert_called_once() + release.assert_called_once() diff --git a/backend/tests/unit/apps/slack/models/reaction_alert_test.py b/backend/tests/unit/apps/slack/models/reaction_alert_test.py index b266694741..3ec6405d6a 100644 --- a/backend/tests/unit/apps/slack/models/reaction_alert_test.py +++ b/backend/tests/unit/apps/slack/models/reaction_alert_test.py @@ -1,63 +1,69 @@ from unittest.mock import Mock +import pytest from django.db import IntegrityError from apps.slack.models.reaction_alert import LOCK_TTL_SECONDS, ReactionAlert +LOCK_OWNER = "lock-owner" +LOCK_KEY = "slack:reaction-alert:7:123.000:spam" + class TestReactionAlert: def test_lock_key(self): """Test lock keys include conversation, message, and report type.""" conversation = Mock(pk=7) - assert ( - ReactionAlert.lock_key(conversation, "123.000", "spam") - == "slack:reaction-alert:7:123.000:spam" - ) + assert ReactionAlert.lock_key(conversation, "123.000", "spam") == LOCK_KEY - def test_acquire_returns_false_when_alert_exists(self, mocker): + def test_acquire_returns_none_when_alert_exists(self, mocker): """Test an existing alert row skips the in-flight lock.""" conversation = Mock(pk=7) manager = mocker.patch("apps.slack.models.reaction_alert.ReactionAlert.objects") manager.filter.return_value.exists.return_value = True add = mocker.patch("apps.slack.models.reaction_alert.cache.add") - assert ReactionAlert.acquire(conversation, "123.000", "spam") is False + assert ReactionAlert.acquire(conversation, "123.000", "spam") is None add.assert_not_called() - def test_acquire_returns_false_when_lock_held(self, mocker): + def test_acquire_returns_none_when_lock_held(self, mocker): """Test a held cache lock skips posting.""" conversation = Mock(pk=7) manager = mocker.patch("apps.slack.models.reaction_alert.ReactionAlert.objects") manager.filter.return_value.exists.return_value = False mocker.patch("apps.slack.models.reaction_alert.cache.add", return_value=False) - assert ReactionAlert.acquire(conversation, "123.000", "spam") is False + assert ReactionAlert.acquire(conversation, "123.000", "spam") is None - def test_acquire_returns_true_when_lock_taken(self, mocker): + def test_acquire_returns_owner_when_lock_taken(self, mocker): """Test a free lock is acquired when no alert row exists.""" conversation = Mock(pk=7) manager = mocker.patch("apps.slack.models.reaction_alert.ReactionAlert.objects") manager.filter.return_value.exists.return_value = False + mocker.patch( + "apps.slack.models.reaction_alert.uuid4", + return_value=Mock(hex=LOCK_OWNER), + ) add = mocker.patch("apps.slack.models.reaction_alert.cache.add", return_value=True) - assert ReactionAlert.acquire(conversation, "123.000", "spam") is True - add.assert_called_once_with( - "slack:reaction-alert:7:123.000:spam", - 1, - timeout=LOCK_TTL_SECONDS, - ) + assert ReactionAlert.acquire(conversation, "123.000", "spam") == LOCK_OWNER + add.assert_called_once_with(LOCK_KEY, LOCK_OWNER, timeout=LOCK_TTL_SECONDS) def test_acquire_releases_lock_if_alert_appears_after_add(self, mocker): - """Test a row created during lock acquisition releases the lock.""" + """Test a row created during lock acquisition releases the owned lock.""" conversation = Mock(pk=7) manager = mocker.patch("apps.slack.models.reaction_alert.ReactionAlert.objects") manager.filter.return_value.exists.side_effect = [False, True] + mocker.patch( + "apps.slack.models.reaction_alert.uuid4", + return_value=Mock(hex=LOCK_OWNER), + ) mocker.patch("apps.slack.models.reaction_alert.cache.add", return_value=True) + mocker.patch("apps.slack.models.reaction_alert.cache.get", return_value=LOCK_OWNER) delete = mocker.patch("apps.slack.models.reaction_alert.cache.delete") - assert ReactionAlert.acquire(conversation, "123.000", "spam") is False - delete.assert_called_once_with("slack:reaction-alert:7:123.000:spam") + assert ReactionAlert.acquire(conversation, "123.000", "spam") is None + delete.assert_called_once_with(LOCK_KEY) def test_exists_for_uses_conversation_message_and_report_type(self, mocker): """Test exists_for looks up the unique alert row.""" @@ -96,9 +102,10 @@ def test_record_creates_alert(self, mocker): ) def test_record_ignores_existing_row(self, mocker): - """Test a concurrent insert does not raise.""" + """Test a concurrent unique insert does not raise.""" manager = mocker.patch("apps.slack.models.reaction_alert.ReactionAlert.objects") manager.create.side_effect = IntegrityError + manager.filter.return_value.exists.return_value = True ReactionAlert.record( Mock(), @@ -109,10 +116,52 @@ def test_record_ignores_existing_row(self, mocker): reporter_user_ids=["U1"], ) - def test_release_deletes_lock(self, mocker): - """Test release deletes the in-flight cache lock.""" + def test_record_raises_unexpected_integrity_error(self, mocker): + """Test a non-unique IntegrityError is not swallowed.""" + manager = mocker.patch("apps.slack.models.reaction_alert.ReactionAlert.objects") + manager.create.side_effect = IntegrityError + manager.filter.return_value.exists.return_value = False + + with pytest.raises(IntegrityError): + ReactionAlert.record( + Mock(), + "123.000", + "spam", + 1, + "999.000", + reporter_user_ids=["U1"], + ) + + def test_release_deletes_owned_lock(self, mocker): + """Test release deletes the lock only when this process still owns it.""" + mocker.patch("apps.slack.models.reaction_alert.cache.get", return_value=LOCK_OWNER) + delete = mocker.patch("apps.slack.models.reaction_alert.cache.delete") + + ReactionAlert.release(Mock(pk=7), "123.000", "spam", LOCK_OWNER) + + delete.assert_called_once_with(LOCK_KEY) + + def test_release_skips_delete_when_owner_mismatch(self, mocker): + """Test release does not delete a lock taken by another process.""" + mocker.patch("apps.slack.models.reaction_alert.cache.get", return_value="other-owner") delete = mocker.patch("apps.slack.models.reaction_alert.cache.delete") - ReactionAlert.release(Mock(pk=7), "123.000", "spam") + ReactionAlert.release(Mock(pk=7), "123.000", "spam", LOCK_OWNER) + + delete.assert_not_called() + + def test_renew_extends_owned_lock(self, mocker): + """Test renew touches the lock when this process still owns it.""" + mocker.patch("apps.slack.models.reaction_alert.cache.get", return_value=LOCK_OWNER) + touch = mocker.patch("apps.slack.models.reaction_alert.cache.touch", return_value=True) + + assert ReactionAlert.renew(Mock(pk=7), "123.000", "spam", LOCK_OWNER) is True + touch.assert_called_once_with(LOCK_KEY, LOCK_TTL_SECONDS) + + def test_renew_returns_false_when_owner_mismatch(self, mocker): + """Test renew does not extend a lock taken by another process.""" + mocker.patch("apps.slack.models.reaction_alert.cache.get", return_value="other-owner") + touch = mocker.patch("apps.slack.models.reaction_alert.cache.touch") - delete.assert_called_once_with("slack:reaction-alert:7:123.000:spam") + assert ReactionAlert.renew(Mock(pk=7), "123.000", "spam", LOCK_OWNER) is False + touch.assert_not_called() diff --git a/backend/tests/unit/apps/slack/models/reaction_rule_test.py b/backend/tests/unit/apps/slack/models/reaction_rule_test.py index 1f102b5082..cf4f5c636a 100644 --- a/backend/tests/unit/apps/slack/models/reaction_rule_test.py +++ b/backend/tests/unit/apps/slack/models/reaction_rule_test.py @@ -1,5 +1,8 @@ from unittest.mock import Mock +import pytest +from django.core.exceptions import ValidationError + from apps.slack.models.conversation import Conversation from apps.slack.models.reaction_rule import ReactionRule from apps.slack.models.workspace import Workspace @@ -7,37 +10,80 @@ class TestReactionRule: def test_str(self): - """Test reaction rule string includes conversation and emoji.""" + """Test reaction rule string includes conversation and emojis.""" workspace = Workspace(name="OWASP") conversation = Conversation(name="general", workspace=workspace) - rule = ReactionRule(conversation=conversation, emoji_name="spam") + rule = ReactionRule(conversation=conversation, emojis=["spam", "flag"]) - assert str(rule) == "OWASP #general :spam" + assert str(rule) == "OWASP #general :spam: :flag:" def test_report_type_defaults_to_spam(self): """Test report type is limited to spam.""" assert ReactionRule.ReportType.SPAM == "spam" assert ReactionRule.ReportType.choices == [("spam", "Spam")] - def test_for_reaction_returns_matching_rule(self, mocker): - """Test reaction rule lookup returns the matching active rule.""" - rule = Mock() + def test_for_emoji_returns_matching_rule(self, mocker): + """Test reaction rule lookup returns the active rule that lists the emoji.""" + rule = Mock(emojis=["spam", "flag"]) manager = mocker.patch("apps.slack.models.reaction_rule.ReactionRule.objects") - manager.select_related.return_value.filter.return_value.first.return_value = rule + manager.select_related.return_value.filter.return_value = [rule] - result = ReactionRule.for_reaction("C123", "spam") + result = ReactionRule.for_emoji("C123", "flag") assert result is rule manager.select_related.assert_called_once_with("conversation") manager.select_related.return_value.filter.assert_called_once_with( conversation__slack_channel_id="C123", - emoji_name="spam", is_active=True, ) - def test_for_reaction_returns_none_when_missing(self, mocker): - """Test reaction rule lookup returns None when no rule exists.""" + def test_for_emoji_returns_none_when_missing(self, mocker): + """Test reaction rule lookup returns None when no rule lists the emoji.""" + manager = mocker.patch("apps.slack.models.reaction_rule.ReactionRule.objects") + manager.select_related.return_value.filter.return_value = [Mock(emojis=["spam"])] + + assert ReactionRule.for_emoji("C123", "flag") is None + + def test_clean_accepts_plain_emojis(self, mocker): + """Test valid emojis are stripped and stored.""" manager = mocker.patch("apps.slack.models.reaction_rule.ReactionRule.objects") - manager.select_related.return_value.filter.return_value.first.return_value = None + manager.filter.return_value = [] + rule = ReactionRule(emojis=[" spam ", "flag"]) + rule.conversation_id = 7 + + rule.clean() + + assert rule.emojis == ["spam", "flag"] + + def test_clean_rejects_empty_emojis(self): + """Test a rule requires at least one emoji.""" + rule = ReactionRule(emojis=[]) + + with pytest.raises(ValidationError, match="at least one"): + rule.clean() + + def test_clean_rejects_colon_wrapped_names(self): + """Test emojis must be stored without Slack colons.""" + rule = ReactionRule(emojis=[":spam:"]) + + with pytest.raises(ValidationError, match="without colons"): + rule.clean() + + def test_clean_rejects_duplicate_names(self): + """Test a rule cannot list the same emoji twice.""" + rule = ReactionRule(emojis=["spam", "spam"]) + + with pytest.raises(ValidationError, match="Duplicate"): + rule.clean() + + def test_clean_rejects_overlapping_emoji_on_same_conversation(self, mocker): + """Test two active rules on one channel cannot share an emoji.""" + manager = mocker.patch("apps.slack.models.reaction_rule.ReactionRule.objects") + manager.filter.return_value = [Mock(emojis=["flag", "x"])] + rule = ReactionRule(emojis=["spam", "flag"]) + rule.conversation_id = 7 + + with pytest.raises(ValidationError, match="already used"): + rule.clean() - assert ReactionRule.for_reaction("C123", "spam") is None + manager.filter.assert_called_once_with(conversation_id=7, is_active=True) diff --git a/backend/tests/unit/apps/slack/utils/reaction_test.py b/backend/tests/unit/apps/slack/utils/reaction_test.py index b64137b474..58dc218fe2 100644 --- a/backend/tests/unit/apps/slack/utils/reaction_test.py +++ b/backend/tests/unit/apps/slack/utils/reaction_test.py @@ -1,4 +1,9 @@ -from apps.slack.utils.reaction import mention_users, parse_message_reaction, reaction_from_payload +from apps.slack.utils.reaction import ( + format_emojis, + mention_users, + parse_message_reaction, + reaction_from_payload, +) EVENT = { "item": {"type": "message", "channel": "C_SOURCE", "ts": "123.000"}, @@ -37,16 +42,58 @@ def test_parse_message_reaction_skips_missing_user(self): class TestReactionFromPayload: def test_reaction_from_payload_returns_matching_emoji(self): - """Test reactions.get payloads expose count, reporters, and permalink.""" - assert reaction_from_payload(PAYLOAD, "spam") == ( + """Test reactions.get payloads expose unique reporters and permalink.""" + assert reaction_from_payload(PAYLOAD, ["spam"]) == ( 2, ["U_REACTOR", "U_OTHER"], "https://slack.test/message", ) def test_reaction_from_payload_returns_none_when_emoji_missing(self): - """Test an unmatched emoji does not produce a snapshot.""" - assert reaction_from_payload(PAYLOAD, "flag") is None + """Test an unmatched emoji set does not produce a snapshot.""" + assert reaction_from_payload(PAYLOAD, ["flag"]) is None + + def test_reaction_from_payload_unions_unique_reporters(self): + """Test listed emojis share one unique-reporter count.""" + payload = { + "message": { + "permalink": "https://slack.test/message", + "reactions": [ + {"name": "spam", "count": 2, "users": ["U1", "U2"]}, + {"name": "flag", "count": 2, "users": ["U2", "U3"]}, + {"name": "thumbsup", "count": 4, "users": ["U4"]}, + ], + } + } + + assert reaction_from_payload(payload, ["spam", "flag"]) == ( + 3, + ["U1", "U2", "U3"], + "https://slack.test/message", + ) + + def test_reaction_from_payload_ignores_non_list_emojis(self): + """Test a JSON scalar emoji list does not produce a snapshot.""" + assert reaction_from_payload(PAYLOAD, "spam") is None + + +class TestFormatEmojis: + def test_format_emojis_joins_names(self): + """Test emoji names are formatted as Slack emoji markup.""" + assert format_emojis(["spam", "flag"]) == ":spam: :flag:" + + def test_format_emojis_handles_empty(self): + """Test missing emoji names produce no markup.""" + assert format_emojis([]) == "" + assert format_emojis(None) == "" + + def test_format_emojis_ignores_non_list_values(self): + """Test JSON scalars and objects are not treated as emoji lists.""" + assert format_emojis("spam") == "" + + def test_format_emojis_ignores_empty_names(self): + """Test blank emoji names are omitted from markup.""" + assert format_emojis(["", "spam", None, "flag"]) == ":spam: :flag:" class TestMentionUsers: @@ -58,3 +105,13 @@ def test_mention_users_handles_empty(self): """Test missing user IDs produce no mention markup.""" assert mention_users([]) == "" assert mention_users(None) == "" + + def test_mention_users_ignores_non_list_values(self): + """Test JSON scalars and objects are not treated as user ID lists.""" + assert mention_users("U123") == "" + assert mention_users({"U123": True}) == "" + assert mention_users(123) == "" + + def test_mention_users_ignores_empty_ids(self): + """Test blank user IDs are omitted from mention markup.""" + assert mention_users(["", "U1", None, "U2"]) == "<@U1> <@U2>" From 7e8146ad013503f27959be73623f880a93c009dd Mon Sep 17 00:00:00 2001 From: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> Date: Sun, 16 Aug 2026 21:19:32 -0700 Subject: [PATCH 10/10] Address comments Signed-off-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> --- backend/src/apps/slack/admin/reaction_rule.py | 1 - .../src/apps/slack/events/reaction_added.py | 9 +++-- ...ionrule_unique_conversation_report_type.py | 24 ++++++++++++ .../src/apps/slack/models/reaction_rule.py | 10 ++++- backend/src/apps/slack/utils/reaction.py | 16 ++++---- .../apps/slack/events/reaction_added_test.py | 39 ++++++++++++++++++- .../apps/slack/models/reaction_rule_test.py | 10 +++++ .../unit/apps/slack/utils/reaction_test.py | 11 ++++++ 8 files changed, 107 insertions(+), 13 deletions(-) create mode 100644 backend/src/apps/slack/migrations/0031_reactionrule_unique_conversation_report_type.py diff --git a/backend/src/apps/slack/admin/reaction_rule.py b/backend/src/apps/slack/admin/reaction_rule.py index bdc5e9b2b1..a8f3752ae4 100644 --- a/backend/src/apps/slack/admin/reaction_rule.py +++ b/backend/src/apps/slack/admin/reaction_rule.py @@ -23,6 +23,5 @@ class ReactionRuleAdmin(admin.ModelAdmin): ) search_fields = ( "conversation__name", - "emojis", "alert_channel_id", ) diff --git a/backend/src/apps/slack/events/reaction_added.py b/backend/src/apps/slack/events/reaction_added.py index 90164e3213..2c51b7e82b 100644 --- a/backend/src/apps/slack/events/reaction_added.py +++ b/backend/src/apps/slack/events/reaction_added.py @@ -2,7 +2,7 @@ import logging -from slack_sdk.errors import SlackApiError +from slack_sdk.errors import SlackApiError, SlackClientError from apps.slack.blocks import markdown from apps.slack.events.event import EventBase @@ -34,6 +34,9 @@ def fetch_permalink(client, channel_id: str, message_ts: str) -> str: e.response.get("error", "unknown_error"), ) return "" + except SlackClientError as e: + logger.warning("Could not fetch Slack permalink for moderation alert: %s", e) + return "" def fetch_reaction(client, channel_id: str, message_ts: str, emojis: list[str]): @@ -74,7 +77,7 @@ def handle_event(self, event, client): if (snapshot := fetch_reaction(client, channel_id, message_ts, rule.emojis)) is None: return - reaction_count, reporter_user_ids, permalink = snapshot + reaction_count, reporter_user_ids, permalink, matched_emojis = snapshot if reaction_count < rule.threshold: return @@ -91,7 +94,7 @@ def handle_event(self, event, client): if ReactionAlert.renew(rule.conversation, message_ts, rule.report_type, owner): alert_users = mention_users(rule.alert_user_ids) reporters = mention_users(reporter_user_ids) - emojis = format_emojis(rule.emojis) + emojis = format_emojis(matched_emojis) text = ( f"{alert_users}\n" f"A message in <#{channel_id}> reached the " diff --git a/backend/src/apps/slack/migrations/0031_reactionrule_unique_conversation_report_type.py b/backend/src/apps/slack/migrations/0031_reactionrule_unique_conversation_report_type.py new file mode 100644 index 0000000000..f82b4ac1fe --- /dev/null +++ b/backend/src/apps/slack/migrations/0031_reactionrule_unique_conversation_report_type.py @@ -0,0 +1,24 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("slack", "0030_alter_reaction_help_text"), + ] + + operations = [ + migrations.AlterUniqueTogether( + name="reactionrule", + unique_together=set(), + ), + migrations.AddConstraint( + model_name="reactionrule", + constraint=models.UniqueConstraint( + fields=("conversation", "report_type"), + name="unique_reactionrule_conversation_report_type", + violation_error_message=( + "A reaction rule already exists for this conversation and report type." + ), + ), + ), + ] diff --git a/backend/src/apps/slack/models/reaction_rule.py b/backend/src/apps/slack/models/reaction_rule.py index a84a8a864b..358549d72e 100644 --- a/backend/src/apps/slack/models/reaction_rule.py +++ b/backend/src/apps/slack/models/reaction_rule.py @@ -20,8 +20,16 @@ class ReportType(models.TextChoices): class Meta: """Model options.""" + constraints = [ + models.UniqueConstraint( + fields=("conversation", "report_type"), + name="unique_reactionrule_conversation_report_type", + violation_error_message=( + "A reaction rule already exists for this conversation and report type." + ), + ), + ] db_table = "slack_reaction_rules" - unique_together = ("conversation", "report_type") alert_channel_id = models.CharField( max_length=50, diff --git a/backend/src/apps/slack/utils/reaction.py b/backend/src/apps/slack/utils/reaction.py index d7652f5745..eb68d9af76 100644 --- a/backend/src/apps/slack/utils/reaction.py +++ b/backend/src/apps/slack/utils/reaction.py @@ -32,8 +32,8 @@ def parse_message_reaction(event): return channel_id, message_ts, emoji_name -def reaction_from_payload(payload, emojis: object) -> tuple[int, list[str], str] | None: - """Return unique reporter IDs and permalink for listed emojis on a reactions.get payload.""" +def reaction_from_payload(payload, emojis: object) -> tuple[int, list[str], str, list[str]] | None: + """Return unique reporters, permalink, and matched emoji names from reactions.get.""" if not isinstance(emojis, list): return None wanted = {name for name in emojis if name} @@ -44,15 +44,17 @@ def reaction_from_payload(payload, emojis: object) -> tuple[int, list[str], str] permalink = message.get("permalink") or "" reporters: list[str] = [] seen: set[str] = set() - matched = False + matched_names: set[str] = set() for reaction in message.get("reactions") or []: - if reaction.get("name") not in wanted: + name = reaction.get("name") + if name not in wanted: continue - matched = True + matched_names.add(name) for user_id in reaction.get("users") or []: if user_id and user_id not in seen: seen.add(user_id) reporters.append(user_id) - if not matched: + if not matched_names: return None - return len(reporters), reporters, permalink + matched_emojis = [name for name in emojis if name in matched_names] + return len(reporters), reporters, permalink, matched_emojis diff --git a/backend/tests/unit/apps/slack/events/reaction_added_test.py b/backend/tests/unit/apps/slack/events/reaction_added_test.py index 3099e1021f..198fe60cc9 100644 --- a/backend/tests/unit/apps/slack/events/reaction_added_test.py +++ b/backend/tests/unit/apps/slack/events/reaction_added_test.py @@ -1,6 +1,6 @@ from unittest.mock import Mock -from slack_sdk.errors import SlackApiError +from slack_sdk.errors import SlackApiError, SlackRequestError from apps.slack.events.reaction_added import ReactionAdded @@ -262,6 +262,22 @@ def test_handle_event_counts_unique_reporters_across_emojis(self, mocker): reporter_user_ids=["U1", "U2", "U3"], ) + def test_handle_event_names_only_matched_emojis(self, mocker): + """Test unused configured emojis are omitted from the alert text.""" + client = mock_client() + rule = mock_rule(threshold=2, emojis=["spam", "flag"]) + patch_rule_lookup(mocker, rule) + patch_alert_lock(mocker) + + ReactionAdded().handle_event(EVENT, client) + + _, kwargs = client.chat_postMessage.call_args + assert ( + "Reported by: <@U_REACTOR> <@U_OTHER> using the following emojis: :spam:" + in kwargs["text"] + ) + assert ":flag:" not in kwargs["text"] + def test_handle_event_skips_non_message_items(self, mocker): """Test file reactions do not look up Slack reactions.""" client = mock_client() @@ -317,3 +333,24 @@ def test_handle_event_posts_when_permalink_unavailable(self, mocker): assert "https://" not in kwargs["text"] record.assert_called_once() release.assert_called_once() + + def test_handle_event_posts_when_permalink_transport_fails(self, mocker): + """Test a permalink transport failure still posts the threshold alert.""" + client = mock_client( + { + "message": { + "reactions": [{"name": "spam", "count": 1, "users": ["U_REACTOR"]}], + } + } + ) + client.chat_getPermalink.side_effect = SlackRequestError("connection failed") + patch_rule_lookup(mocker) + _, release, record = patch_alert_lock(mocker) + + ReactionAdded().handle_event(EVENT, client) + + client.chat_postMessage.assert_called_once() + _, kwargs = client.chat_postMessage.call_args + assert "https://" not in kwargs["text"] + record.assert_called_once() + release.assert_called_once() diff --git a/backend/tests/unit/apps/slack/models/reaction_rule_test.py b/backend/tests/unit/apps/slack/models/reaction_rule_test.py index cf4f5c636a..9e6ce8354f 100644 --- a/backend/tests/unit/apps/slack/models/reaction_rule_test.py +++ b/backend/tests/unit/apps/slack/models/reaction_rule_test.py @@ -22,6 +22,16 @@ def test_report_type_defaults_to_spam(self): assert ReactionRule.ReportType.SPAM == "spam" assert ReactionRule.ReportType.choices == [("spam", "Spam")] + def test_unique_conversation_report_type_constraint(self): + """Test one reaction rule is allowed per conversation and report type.""" + constraint = next( + item + for item in ReactionRule._meta.constraints + if item.name == "unique_reactionrule_conversation_report_type" + ) + + assert tuple(constraint.fields) == ("conversation", "report_type") + def test_for_emoji_returns_matching_rule(self, mocker): """Test reaction rule lookup returns the active rule that lists the emoji.""" rule = Mock(emojis=["spam", "flag"]) diff --git a/backend/tests/unit/apps/slack/utils/reaction_test.py b/backend/tests/unit/apps/slack/utils/reaction_test.py index 58dc218fe2..8966f5d7d4 100644 --- a/backend/tests/unit/apps/slack/utils/reaction_test.py +++ b/backend/tests/unit/apps/slack/utils/reaction_test.py @@ -47,6 +47,7 @@ def test_reaction_from_payload_returns_matching_emoji(self): 2, ["U_REACTOR", "U_OTHER"], "https://slack.test/message", + ["spam"], ) def test_reaction_from_payload_returns_none_when_emoji_missing(self): @@ -70,6 +71,16 @@ def test_reaction_from_payload_unions_unique_reporters(self): 3, ["U1", "U2", "U3"], "https://slack.test/message", + ["spam", "flag"], + ) + + def test_reaction_from_payload_returns_matched_emojis_only(self): + """Test unused configured emojis are omitted from the snapshot.""" + assert reaction_from_payload(PAYLOAD, ["spam", "flag"]) == ( + 2, + ["U_REACTOR", "U_OTHER"], + "https://slack.test/message", + ["spam"], ) def test_reaction_from_payload_ignores_non_list_emojis(self):