Merge remote-tracking branch 'origin/develop' into async

apps/archive/archive.py

@@ -1236,7 +1236,7 @@ def handle_mark_user_notifications(self, updates, original, add_activity=True):
             )
 
             self._send_mark_user_notifications(
-                "item:unmarked",
+                "item:marked",
                 message,
                 resource=self.datasource,
                 item=original,
@@ -1298,9 +1298,9 @@ def _send_mark_user_notifications(
                 item=item,
                 user_list=user_list,
                 link=link,
-                preference_notification_name="mark_for_user",
                 **data,
             )
+
         # send separate notification for markForUser extension
         push_notification(activity_name, item_id=item.get(ID_FIELD), user_list=user_list, extension="markForUser")
 

apps/preferences.py

@@ -11,12 +11,11 @@
 import logging
 import superdesk
 
-from superdesk.resource_fields import ID_FIELD
 from superdesk.flask import request
-from superdesk.resource import Resource
-from superdesk.services import BaseService
-from superdesk import get_backend
-from superdesk import get_resource_service
+from superdesk.resource_fields import ID_FIELD
+from superdesk.preferences import get_user_notification_preferences
+from superdesk.resource import Resource, BaseService
+from superdesk import get_backend, get_resource_service
 from superdesk.workflow import get_privileged_actions
 from superdesk.validation import ValidationError
 from flask_babel import _, lazy_gettext
@@ -212,17 +211,6 @@ class PreferencesResource(Resource):
         category=lazy_gettext("contacts"),
     )
 
-    superdesk.register_default_user_preference(
-        "email:notification:mark_for_user",
-        {
-            "type": "bool",
-            "enabled": True,
-            "default": True,
-        },
-        label=lazy_gettext("Send Mark for User notifications via email"),
-        category=lazy_gettext("notifications"),
-    )
-
     superdesk.register_default_user_preference("destination:active", {})
 
     superdesk.register_default_user_preference("extensions", {})
@@ -354,14 +342,16 @@ def get_user_preference(self, user_id):
         prefs = doc.get(_user_preferences_key, {})
         return prefs
 
-    def email_notification_is_enabled(self, user_id=None, preferences=None):
+    def email_notification_is_enabled(self, user_id=None) -> bool:
         """
         This function checks if email notification is enabled or not based on the preferences.
         """
         if user_id:
-            preferences = self.get_user_preference(user_id)
-        send_email = preferences.get("email:notification", {}) if isinstance(preferences, dict) else {}
-        return send_email and send_email.get("enabled", False)
+            user = get_resource_service("users").find_one(req=None, _id=user_id)
+            if not user:
+                return False
+            return get_user_notification_preferences(user)["email"]
+        return False
 
     def is_authorized(self, **kwargs):
         """
@@ -411,16 +401,3 @@ def has_missing_privileges(prefs):
             return [priv for pref in prefs for priv in pref.get("privileges", []) if not privileges.get(priv)]
 
         doc[_user_preferences_key] = {k: v for k, v in preferences.items() if not has_missing_privileges(v)}
-
-    def check_preference_email_notification_is_enabled(self, preference_name, user_id=None, preferences=None):
-        """
-        This function checks if email notification is enabled or not based on the preference.
-        """
-        send_email = {}
-        if user_id:
-            preferences = self.get_user_preference(user_id)
-        if preference_name:
-            send_email = (
-                preferences.get(f"email:notification:{preference_name}", {}) if isinstance(preferences, dict) else {}
-            )
-        return send_email and send_email.get("enabled", False)

docs/database recording.rst

@@ -60,7 +60,7 @@ terminal, in the directory where your ``docker-compose.yml`` is located.
 
 ::
 
-   > docker-compose exec mongodb /bin/bash
+   > docker compose exec mongodb /bin/bash
    > mongo
    > rs.initiate()
 

scripts/tests_setup

@@ -11,6 +11,6 @@ sudo dpkg -i libmagic-mgc_5.37-3_amd64.deb
 sudo dpkg -i libmagic1_5.37-3_amd64.deb
 
 if [ "$SKIP_SERVER_INSTALL" != "true" ]; then
-    docker-compose up -d
+    docker compose up -d
     while ! curl -sfo /dev/null 'http://localhost:9200/'; do echo -n '.' && sleep .5; done
 fi

superdesk/activity.py

@@ -10,6 +10,7 @@
 
 import datetime
 import logging
+from typing import List
 
 from bson.objectid import ObjectId
 from flask_babel import _, lazy_gettext
@@ -20,8 +21,10 @@
 from superdesk.emails import send_activity_emails
 from superdesk.errors import SuperdeskApiError, add_notifier
 from superdesk.notification import push_notification
+from superdesk.preferences import get_user_notification_preferences
 from superdesk.resource import Resource
 from superdesk.services import BaseService
+from superdesk.types import User
 from superdesk.utc import utcnow
 from eve.utils import ParsedRequest
 from superdesk.metadata.item import PUBLISH_STATES
@@ -80,6 +83,13 @@ class ActivityResource(Resource):
         label=lazy_gettext("Send notifications via email"),
         category=lazy_gettext("notifications"),
     )
+    superdesk.register_default_user_preference(
+        "notifications",
+        {
+            "type": "dict",
+            "schema": {},
+        },
+    )
     superdesk.register_default_user_preference(
         "desktop:notification",
         {
@@ -229,23 +239,37 @@ def add_activity(
 
 
 def notify_and_add_activity(
-    activity_name, msg, resource=None, item=None, user_list=None, preference_notification_name=None, **data
+    activity_name, msg, resource=None, item=None, user_list=None, notification_name=None, **data
 ):
     """
     Adds the activity and notify enabled and active users via email.
     """
+    if user_list is None:
+        user_list = []
+
+    if notification_name is None:
+        notification_name = activity_name
+
+    notify_users = [
+        str(user.get("_id"))
+        for user in user_list
+        if get_user_notification_preferences(user, notification_name)["desktop"]
+    ]
 
     add_activity(
         activity_name,
         msg=msg,
         resource=resource,
         item=item,
-        notify=[str(user.get("_id")) for user in user_list] if user_list else None,
+        notify=notify_users,
         **data,
     )
 
     if activity_name == ACTIVITY_ERROR or user_list:
-        recipients = get_recipients(user_list, activity_name, preference_notification_name)
+        if not user_list:
+            user_list = get_resource_service("users").get_users_by_user_type("administrator")
+
+        recipients = get_recipients(user_list, notification_name)
 
         if activity_name != ACTIVITY_ERROR:
             current_user = get_current_app().get_current_user_dict()
@@ -262,25 +286,14 @@ def notify_and_add_activity(
             send_activity_emails(activity=activity, recipients=recipients)
 
 
-def get_recipients(user_list, activity_name, preference_notification_name=None):
-    if not user_list and activity_name == ACTIVITY_ERROR:
-        user_list = get_resource_service("users").get_users_by_user_type("administrator")
-
+def get_recipients(user_list: List[User], notification_name=None) -> List[str]:
     recipients = [
-        user.get("email")
+        user["email"]
         for user in user_list
         if not user.get("needs_activation", True)
         and user.get("is_enabled", False)
         and user.get("is_active", False)
-        and get_resource_service("preferences").email_notification_is_enabled(
-            preferences=user.get("user_preferences", {})
-        )
-        and (
-            not preference_notification_name
-            or get_resource_service("preferences").check_preference_email_notification_is_enabled(
-                preference_notification_name, preferences=user.get("user_preferences", {})
-            )
-        )
+        and get_user_notification_preferences(user, notification_name)["email"]
     ]
 
     return recipients

superdesk/preferences.py

@@ -0,0 +1,43 @@
+from typing import Optional, TypedDict
+from superdesk.types import User
+
+
+class NotificationPreferences(TypedDict):
+    email: bool
+    desktop: bool
+
+
+def get_user_notification_preferences(user: User, notification: Optional[str] = None) -> NotificationPreferences:
+    user_preferences = user.get("user_preferences") or {}
+
+    def is_enabled(preference: str) -> bool:
+        return bool(user_preferences.get(preference, {}).get("enabled", False))
+
+    email_enabled = is_enabled("email:notification")
+    desktop_enabled = is_enabled("desktop:notification")
+
+    if notification is None:
+        return NotificationPreferences(
+            email=email_enabled,
+            desktop=desktop_enabled,
+        )
+
+    notification_preferences = user_preferences.get("notifications", {}).get(notification)
+    if notification_preferences is None:
+        # BC: Check for the old email:notification:<notification> preference
+        email_notification_name = f"email:notification:{notification}"
+        if user_preferences.get(email_notification_name):
+            return NotificationPreferences(
+                email=email_enabled and is_enabled(email_notification_name),
+                desktop=desktop_enabled,
+            )
+        else:
+            return NotificationPreferences(
+                email=email_enabled,
+                desktop=desktop_enabled,
+            )
+
+    return NotificationPreferences(
+        email=email_enabled and notification_preferences.get("email") is not False,
+        desktop=desktop_enabled and notification_preferences.get("desktop") is not False,
+    )

superdesk/types.py

@@ -28,3 +28,15 @@ class Item(TypedDict, total=False):
     slugline: str
     authors: List[ItemAuthor]
     extra: Dict[str, Any]
+
+
+class UserMandatory(TypedDict):
+    email: str
+    username: str
+
+
+class User(UserMandatory, total=False):
+    user_preferences: Dict[str, Any]
+    needs_activation: bool
+    is_enabled: bool
+    is_active: bool

tests/activity_tests.py

@@ -8,22 +8,10 @@
 # AUTHORS and LICENSE files distributed with this source code, or
 # at https://www.sourcefabric.org/superdesk/license
 
-from unittest import mock
-
-from superdesk.activity import ActivityService, ACTIVITY_ERROR, get_recipients
+from superdesk.activity import ActivityService, get_recipients
 from superdesk.publish import init_app
 from superdesk.tests import TestCase
-
-
-def mock_get_resource_service(resource_name):
-    return MockPreferenceService()
-
-
-class MockPreferenceService:
-    def email_notification_is_enabled(self, preferences=None):
-        send_email = preferences.get("email:notification", {}) if isinstance(preferences, dict) else {}
-
-        return send_email and send_email.get("enabled", False)
+from superdesk.types import User
 
 
 class ActivityTestCase(TestCase):
@@ -44,24 +32,51 @@ def test_is_read(self):
         self.assertFalse(ActivityService().is_read(activity, "2"))
         self.assertFalse(ActivityService().is_read(activity, "3"))
 
-    @mock.patch("superdesk.activity.get_resource_service", mock_get_resource_service)
     def test_get_recipients_filters_out_users_not_activated(self):
         users = [
-            {
-                "email": "[email protected]",
-                "needs_activation": False,
-                "is_enabled": True,
-                "is_active": True,
-                "user_preferences": {"email:notification": {"enabled": True}},
-            },
-            {
-                "email": "[email protected]",
-                "needs_activation": True,
-                "is_enabled": True,
-                "is_active": True,
-                "user_preferences": {"email:notification": {"enabled": True}},
-            },
+            User(
+                {
+                    "username": "test1",
+                    "email": "[email protected]",
+                    "needs_activation": False,
+                    "is_enabled": True,
+                    "is_active": True,
+                    "user_preferences": {"email:notification": {"enabled": True}},
+                }
+            ),
+            User(
+                {
+                    "username": "test2",
+                    "email": "[email protected]",
+                    "needs_activation": True,
+                    "is_enabled": True,
+                    "is_active": True,
+                    "user_preferences": {"email:notification": {"enabled": True}},
+                }
+            ),
         ]
 
-        recipients = get_recipients(user_list=users, activity_name=ACTIVITY_ERROR)
+        recipients = get_recipients(user_list=users)
         self.assertEqual(len(recipients), 1)
+        self.assertEqual(recipients[0], "[email protected]")
+
+    def test_get_recipients_filters_out_users_with_disabled_notification(self):
+        users = [
+            User(
+                {
+                    "username": "test1",
+                    "email": "[email protected]",
+                    "needs_activation": False,
+                    "is_enabled": True,
+                    "is_active": True,
+                    "user_preferences": {"email:notification": {"enabled": False}},
+                }
+            ),
+        ]
+
+        recipients = get_recipients(user_list=users, notification_name="test")
+        assert len(recipients) == 0
+
+        users[0]["user_preferences"]["email:notification"]["enabled"] = True
+        recipients = get_recipients(user_list=users, notification_name="test")
+        assert len(recipients) == 1