Merge pull request #17975 from davelopez/make_urgent_notifications_ma…

…ndatory

Make urgent notifications mandatory

client/src/components/User/Notifications/NotificationsPreferences.vue

@@ -48,8 +48,12 @@ const showPreferences = computed(() => {
 });
 
 const categoryDescriptionMap: Record<string, string> = {
-    message: "You will receive notifications when someone sends you a message.",
-    new_shared_item: "You will receive notifications when someone shares an item with you.",
+    message: `
+        You will receive these notifications only when your Galaxy administrators send you a message.
+        Please note that for certain critical or urgent messages, you will receive notifications even if you have disabled this channel.
+    `,
+    new_shared_item:
+        "You will receive these notifications when someone shares an item with you i.e. a history, workflow, visualization, etc.",
 };
 
 async function getNotificationsPreferences() {
@@ -131,14 +135,6 @@ watch(
         <div v-else-if="showPreferences" class="notifications-preferences-body">
             <div v-for="category in categories" :key="category" class="card-container">
                 <div class="category-header">
-                    <div>
-                        <div v-localize class="category-title">{{ capitalizeWords(category) }}</div>
-
-                        <div v-if="categoryDescriptionMap[category]" v-localize class="category-description">
-                            {{ categoryDescriptionMap[category] }}
-                        </div>
-                    </div>
-
                     <BFormCheckbox
                         v-model="notificationsPreferences[category].enabled"
                         v-b-tooltip.hover
@@ -147,7 +143,13 @@ watch(
                                 ? 'Disable notifications'
                                 : 'Enable notifications'
                         "
-                        switch />
+                        switch>
+                        <span v-localize class="category-title">{{ capitalizeWords(category) }}</span>
+                    </BFormCheckbox>
+                </div>
+
+                <div v-if="categoryDescriptionMap[category]" v-localize class="category-description">
+                    {{ categoryDescriptionMap[category] }}
                 </div>
 
                 <div v-for="channel in supportedChannels" :key="channel" class="category-channel">
@@ -244,5 +246,10 @@ watch(
 .category-description {
     font-size: 0.8rem;
     font-style: italic;
+    margin-bottom: 0.5rem;
+}
+
+.card-container {
+    margin: 0.5rem;
 }
 </style>

client/src/components/admin/Notifications/NotificationForm.vue

@@ -89,6 +89,8 @@ const expirationDate = computed({
     },
 });
 
+const isUrgent = computed(() => notificationData.value.notification.variant === "urgent");
+
 async function loadData<T>(
     getData: () => Promise<T[]>,
     target: Ref<SelectOption[]>,
@@ -161,13 +163,20 @@ async function sendNewNotification() {
                 type="select"
                 title="Variant"
                 :optional="false"
-                help="This will change the color of the notification"
+                help="This measures the urgency of the notification and will affect the color of the notification."
                 :options="[
                     ['Info', 'info'],
                     ['Warning', 'warning'],
                     ['Urgent', 'urgent'],
                 ]" />
 
+            <BAlert :show="isUrgent" variant="warning">
+                <span v-localize>
+                    Urgent notifications will ignore the user's notification preferences and will be sent to all
+                    available channels. Please use this option sparingly and only for critical notifications.
+                </span>
+            </BAlert>
+
             <FormElement
                 id="notification-recipients-user-ids"
                 v-model="notificationData.recipients.user_ids"

lib/galaxy/config/templates/mail/notifications/message-email.html

@@ -16,9 +16,10 @@
 - hostname                  Your galaxy's hostname (i.e. usegalaxy.* or the value in `server_name` from the galaxy config file)
 - contact_email             Your galaxy's contact email
 - notification_settings_url The URL to the user's notification settings to manage their subscriptions
+- variant                   The notification variant indicates the level of importance of the notification (i.e. info, warning, urgent)
 - content                   The message payload
-  - subject                   The message subject
-  - content                   The message content in HTML (converted from Markdown)
+  - subject                 The message subject
+  - content                 The message content in HTML (converted from Markdown)
 - galaxy_url                The URL to the Galaxy instance (i.e. https://usegalaxy.*)
 
 Template begins here >>>>>>

lib/galaxy/config/templates/mail/notifications/message-email.txt

@@ -16,6 +16,7 @@ sent:
 - hostname                  Your galaxy's hostname (i.e. usegalaxy.* or the value in `server_name` from the galaxy config file)
 - contact_email             Your galaxy's contact email
 - notification_settings_url The URL to the user's notification settings to manage their subscriptions
+- variant                   The notification variant indicates the level of importance of the notification (i.e. info, warning, urgent)
 - content                   The message payload
   - subject                 The message subject
   - content                 The message content in HTML (converted from Markdown)

lib/galaxy/config/templates/mail/notifications/new_shared_item-email.html

@@ -16,6 +16,7 @@
 - hostname                  Your galaxy's hostname (i.e. usegalaxy.* or the value in `server_name` from the galaxy config file)
 - contact_email             Your galaxy's contact email
 - notification_settings_url The URL to the user's notification settings to manage their subscriptions
+- variant                   The notification variant indicates the level of importance of the notification (i.e. info, warning, urgent)
 - content                   The new_shared_item payload
   - item_type               The type of the shared item
   - item_name               The name of the shared item

lib/galaxy/config/templates/mail/notifications/new_shared_item-email.txt

@@ -16,6 +16,7 @@ sent:
 - hostname                  Your galaxy's hostname (i.e. usegalaxy.* or the value in `server_name` from the galaxy config file)
 - contact_email             Your galaxy's contact email
 - notification_settings_url The URL to the user's notification settings to manage their subscriptions
+- variant                   The notification variant indicates the level of importance of the notification (i.e. info, warning, urgent)
 - content                   The new_shared_item payload
   - item_type               The type of the shared item
   - item_name               The name of the shared item

lib/galaxy/managers/notification.py

@@ -31,6 +31,7 @@
 from sqlalchemy.sql import Select
 from typing_extensions import Protocol
 
+from galaxy import util
 from galaxy.config import (
     GalaxyAppConfiguration,
     templates,
@@ -62,15 +63,12 @@
     NotificationCreateData,
     NotificationCreateRequest,
     NotificationRecipients,
+    NotificationVariant,
     PersonalNotificationCategory,
     UpdateUserNotificationPreferencesRequest,
     UserNotificationPreferences,
     UserNotificationUpdateRequest,
 )
-from galaxy.util import (
-    send_mail,
-    unicodify,
-)
 
 log = logging.getLogger(__name__)
 
@@ -179,12 +177,12 @@ def _create_associations(self, notification: Notification, users: List[User]) ->
         success_count = 0
         for user in users:
             try:
-                if self._is_user_subscribed_to_category(user, notification.category):  # type:ignore[arg-type]
+                if self._is_user_subscribed_to_notification(user, notification):
                     user_notification_association = UserNotificationAssociation(user, notification)
                     self.sa_session.add(user_notification_association)
                     success_count += 1
             except Exception as e:
-                log.error(f"Error sending notification to user {user.id}. Reason: {unicodify(e)}")
+                log.error(f"Error sending notification to user {user.id}. Reason: {util.unicodify(e)}")
                 continue
         return success_count
 
@@ -221,10 +219,13 @@ def get_pending_notifications(self):
     def _dispatch_notification_to_users(self, notification: Notification):
         users = self._get_associated_users(notification)
         for user in users:
-            category_settings = self._get_user_category_settings(user, notification.category)  # type:ignore[arg-type]
-            if not self._is_subscribed_to_category(category_settings):
+            try:
+                if self._is_user_subscribed_to_notification(user, notification):
+                    settings = self._get_user_category_settings(user, notification.category)  # type:ignore[arg-type]
+                    self._send_via_channels(notification, user, settings.channels)
+            except Exception as e:
+                log.error(f"Error sending notification to user {user.id}. Reason: {util.unicodify(e)}")
                 continue
-            self._send_via_channels(notification, user, category_settings.channels)
 
     def _get_associated_users(self, notification: Notification):
         stmt = (
@@ -239,24 +240,35 @@ def _get_associated_users(self, notification: Notification):
         )
         return self.sa_session.execute(stmt).scalars().all()
 
-    def _is_user_subscribed_to_category(self, user: User, category: PersonalNotificationCategory) -> bool:
-        category_settings = self._get_user_category_settings(user, category)
+    def _is_user_subscribed_to_notification(self, user: User, notification: Notification) -> bool:
+        if self._is_urgent(notification):
+            # Urgent notifications are always sent
+            return True
+        category_settings = self._get_user_category_settings(user, notification.category)  # type:ignore[arg-type]
         return self._is_subscribed_to_category(category_settings)
 
     def _send_via_channels(self, notification: Notification, user: User, channel_settings: NotificationChannelSettings):
         channels = channel_settings.model_fields_set
         for channel in channels:
             if channel not in self.channel_plugins:
-                log.warning(f"Notification channel '{channel}' is not supported.")
-                continue
-            if getattr(channel_settings, channel, False) is False:
-                continue  # User opted out of this channel
-            plugin = self.channel_plugins[channel]
-            plugin.send(notification, user)
+                continue  # Skip unsupported channels
+            user_opted_out = getattr(channel_settings, channel, False) is False
+            if user_opted_out and not self._is_urgent(notification):
+                continue  # Skip sending to opted-out users unless it's an urgent notification
+            try:
+                plugin = self.channel_plugins[channel]
+                plugin.send(notification, user)
+            except Exception as e:
+                log.error(
+                    f"Error sending notification to user {user.id} via channel '{channel}'. Reason: {util.unicodify(e)}"
+                )
 
     def _is_subscribed_to_category(self, category_settings: NotificationCategorySettings) -> bool:
         return category_settings.enabled
 
+    def _is_urgent(self, notification: Notification) -> bool:
+        return notification.variant == NotificationVariant.urgent.value
+
     def _get_user_category_settings(
         self, user: User, category: PersonalNotificationCategory
     ) -> NotificationCategorySettings:
@@ -652,6 +664,7 @@ class NotificationContext(BaseModel):
     date: str
     hostname: str
     contact_email: str
+    variant: str
     notification_settings_url: str
     content: AnyNotificationContent
     galaxy_url: Optional[str] = None
@@ -701,6 +714,7 @@ def build_context(self, template_format: TemplateFormats) -> NotificationContext
             hostname=hostname,
             contact_email=contact_email,
             notification_settings_url=notification_settings_url,
+            variant=notification.variant,
             content=self.get_content(template_format),
             galaxy_url=self.notification.galaxy_url,
         )
@@ -762,7 +776,7 @@ def send(self, notification: Notification, user: User):
             subject = template_builder.get_subject()
             text_body = template_builder.get_body(TemplateFormats.TXT)
             html_body = template_builder.get_body(TemplateFormats.HTML)
-            send_mail(
+            util.send_mail(
                 frm=self.config.email_from,
                 to=user.email,
                 subject=subject,
@@ -771,5 +785,5 @@ def send(self, notification: Notification, user: User):
                 html=html_body,
             )
         except Exception as e:
-            log.error(f"Error sending email notification to user {user.id}. Reason: {unicodify(e)}")
+            log.error(f"Error sending email notification to user {user.id}. Reason: {util.unicodify(e)}")
             pass

test/unit/app/managers/test_NotificationManager.py

@@ -9,6 +9,7 @@
     Optional,
     Set,
 )
+from unittest.mock import patch
 
 import pytest
 
@@ -113,6 +114,13 @@ def _assert_notification_expected(self, actual_notification: Any, expected_notif
         assert user_notification_content["message"] == expected_notification["content"]["message"]
 
 
+class NotificationManagerBaseTestCaseWithTasks(NotificationManagerBaseTestCase):
+    def set_up_managers(self):
+        super().set_up_managers()
+        self.app.config.enable_celery_tasks = True
+        self.notification_manager = NotificationManager(self.trans.sa_session, self.app.config)
+
+
 class TestBroadcastNotifications(NotificationManagerBaseTestCase):
     def test_create_notification_broadcast(self):
         data = self._default_broadcast_notification_data()
@@ -369,6 +377,79 @@ def test_users_do_not_receive_opt_out_notifications(self):
         user_notifications = self.notification_manager.get_user_notifications(user_opt_in)
         assert len(user_notifications) == 1
 
+    def test_urgent_notifications_ignore_preferences(self):
+        user = self._create_test_user()
+        update_request = UpdateUserNotificationPreferencesRequest(
+            preferences={
+                PersonalNotificationCategory.message: NotificationCategorySettings(enabled=False),
+            }
+        )
+        self.notification_manager.update_user_notification_preferences(user, update_request)
+
+        # Send normal message notification
+        notification_data = self._default_test_notification_data()
+        self._send_message_notification_to_users([user], notification=notification_data)
+        user_notifications = self.notification_manager.get_user_notifications(user)
+        assert len(user_notifications) == 0
+
+        # Send urgent message notification
+        notification_data["variant"] = NotificationVariant.urgent
+        self._send_message_notification_to_users([user], notification=notification_data)
+        user_notifications = self.notification_manager.get_user_notifications(user)
+        assert len(user_notifications) == 1
+
+
+class TestUserNotificationsWithTasks(NotificationManagerBaseTestCaseWithTasks):
+
+    def test_urgent_notifications_via_email_channel(self):
+        user = self._create_test_user()
+        # Disable email channel only
+        update_request = UpdateUserNotificationPreferencesRequest(
+            preferences={
+                PersonalNotificationCategory.message: NotificationCategorySettings(
+                    enabled=True,
+                    channels=NotificationChannelSettings(email=False),
+                ),
+            }
+        )
+        self.notification_manager.update_user_notification_preferences(user, update_request)
+
+        emails_sent = []
+
+        def validate_send_email(frm, to, subject, body, config, html=None):
+            emails_sent.append(
+                {
+                    "frm": frm,
+                    "to": to,
+                    "subject": subject,
+                    "body": body,
+                    "config": config,
+                    "html": html,
+                }
+            )
+
+        with patch("galaxy.util.send_mail", side_effect=validate_send_email) as mock_send_mail:
+            notification_data = self._default_test_notification_data()
+
+            # Send normal message notification
+            self._send_message_notification_to_users([user], notification=notification_data)
+            # The in-app notification should be sent but the email notification should not be sent after dispatching
+            user_notifications = self.notification_manager.get_user_notifications(user)
+            assert len(user_notifications) == 1
+            self.notification_manager.dispatch_pending_notifications_via_channels()
+            mock_send_mail.assert_not_called()
+
+            # Send urgent message notification
+            notification_data["variant"] = NotificationVariant.urgent
+            self._send_message_notification_to_users([user], notification=notification_data)
+            # The in-app notification should be sent, now there are 2 notifications
+            user_notifications = self.notification_manager.get_user_notifications(user)
+            assert len(user_notifications) == 2
+            # The email notification should be sent after dispatching the pending notifications
+            self.notification_manager.dispatch_pending_notifications_via_channels()
+            mock_send_mail.assert_called_once()
+            assert len(emails_sent) == 1
+
 
 class TestNotificationRecipientResolver(NotificationsBaseTestCase):
     def test_default_resolution_strategy(self):