diff --git a/changelog.d/+cleanup-destination-plugins.changed.md b/changelog.d/+cleanup-destination-plugins.changed.md
new file mode 100644
index 000000000..252ad3857
--- /dev/null
+++ b/changelog.d/+cleanup-destination-plugins.changed.md
@@ -0,0 +1,6 @@
+Moved a lot of common infrastructure from our NotificationMedium subclasses to
+the parent. This should make it easier to create more media of high quality.
+Also, it should make it easier to use the plugins for validating the
+settings-file elsewhere than just the API.
+
+This might break 3rd party notification plugins.
diff --git a/docs/integrations/notifications/writing-notification-plugins.rst b/docs/integrations/notifications/writing-notification-plugins.rst
index eb4bacd40..cb98d4ab5 100644
--- a/docs/integrations/notifications/writing-notification-plugins.rst
+++ b/docs/integrations/notifications/writing-notification-plugins.rst
@@ -28,22 +28,39 @@ implement the following:
Class constants
---------------
-You need to set the constants `MEDIA_SLUG`, `MEDIA_NAME` and
-`MEDIA_JSON_SCHEMA`.
-
-The media name is the name of the service you want to send notifications by.
-This is used only for display purposes so you might want to keep it short and
-sweet. So for example `Email`, `SMS` or `MS Teams`.
-
-The media slug is the slugified version of that, so the name simplified to only
-contain lowercase letters, numbers, underscores and hyphens. Always have it
-start with a letter, a-z. For example `email`, `sms` or `msteams`.
-
-The media `json schema `_ is a representation of how
-a destination that will be used by this notification plugin should look like.
-Such a destination should include all necessary information that is needed to
-send notifications with your notification plugin. In case of SMS that is a
-phone number or for MS Teams a webhook.
+You must set the constants ``MEDIA_SLUG`, `MEDIA_NAME`` and
+``MEDIA_JSON_SCHEMA``. If your plugin only takes or needs a single
+configuration flag you should also set ``MEDIA_SETTINGS_KEY``.
+
+MEDIA_NAME
+ The media name is the name of the service you want to send notifications by.
+ This is used only for display purposes so you might want to keep it short
+ and sweet. So for example ``"Email"``, ``"SMS"`` or ``"MS Teams"``.
+
+MEDIA_SLUG
+ The media slug is the slugified version of that, so the name simplified to
+ only contain lowercase letters, numbers, underscores and hyphens. Always
+ have it start with a letter, a-z. For example ``"email"``, ``"sms"`` or
+ ``"msteams"``.
+
+MEDIA_JSON_SCHEMA
+ The media `json schema `_ is a representation of
+ how a destination that will be used by this notification plugin should look
+ like, so that it is possible to autogenerate a form with javascript. It will
+ be accessible via the API. Such a destination should include all necessary
+ information that is needed to send notifications with your notification
+ plugin. In case of SMS that is a phone number or for MS Teams a webhook.
+
+MEDIA_SETTINGS_KEY
+ The media settings key is the name of the most important key in the settings
+ JSON field. It is used to cut down on the amount of code you need to write
+ if there is only one piece of data you need to send the notification. Among
+ other things, it is used to check for duplicate entries, so in a way it acts
+ as the primary key for your plugin. For that reason, it must be required in
+ the json schema.
+
+Form
+ The ``forms.Form`` used to validate the settings-field.
Class methods for sending notifications
---------------------------------------
@@ -51,6 +68,8 @@ Class methods for sending notifications
.. autoclass:: argus.notificationprofile.media.base.NotificationMedium
:members: send
+This MUST be overridden.
+
The ``send`` method is the method that does the actual sending of the
notification. It gets the Argus event and a list of destinations as input and
returns a boolean indicating if the sending was successful.
@@ -62,35 +81,67 @@ The rest is very dependent on the notification medium and, if used, the Python
library. The given event can be used to extract relevant information that
should be included in the message that will be sent to each destination.
-Class methods for destinations
-------------------------------
+Helper class methods
+--------------------
.. autoclass:: argus.notificationprofile.media.base.NotificationMedium
:members: get_label, has_duplicate, raise_if_not_deletable, update, validate
:noindex:
-Your implementation of ``get_label`` should show a reasonable representation
-for a destination of that type that makes it easy to identify. For SMS that
-would simply be the phone number.
-
-The method ``has_duplicate`` will receive a QuerySet of destinations and a dict
-of settings for a possible destination and should return True if a destination
-with such settings exists in the given QuerySet.
-
-``raise_if_not_deletable`` should check if a given destination can be deleted.
-This is used in case some destinations are synced from an outside source and
-should not be able to be deleted by a user. If that is the case a
-``NotDeletableError`` should be raised. If not simply return None.
-
-The method ``update`` only has to be implemented if the regular update method
-of Django isn't sufficient. This can be the case if additional settings need to
-be updated.
-
-Finally the function ``validate`` makes sure that a destination with the given
-settings can be updated or created. The function ``has_duplicate`` can be used
-here to ensure that not two destinations with the same settings will be
-created. Additionally the settings themselves should also be validated. For
-example for SMS the given phone number will be checked. Django forms can be
-helpful for validation. A ``ValidationError`` should be raised if the given
-settings are invalid and the validated and cleaned data should be returned if
-not.
+With a little luck you might not need to override any of these.
+
+clean
+ This method will do any additional cleaning beyond what is defined by the
+ defined ``Form``. Expects a valid form instance and returns the updated
+ valid form instance.
+
+get_label
+ Your implementation of ``get_label`` should show a reasonable representation
+ for a destination of that type that makes it easy to identify. For SMS that
+ would simply be the phone number. By default it shows the label stored in
+ the destination. If no label have been set, it uses MEDIA_SETTINGS_KEY to
+ look up the most important piece of information in the settings and uses
+ that directly. The included plugins need not override ``get_label`` for this
+ reason. If the label would be very long, for instance if the needed setting
+ is a very long url (40+ characters), you ought to write your own
+ ``get_label``.
+
+get_relevant_destination_settings
+ Used by ``send``. You should only need to override this if the key in
+ MEDIA_SETTINGS_KEY is insuffcient to look up the actual configuration of the
+ destinations of the type set by MEDIA_SLUG.
+
+has_duplicate
+ The method ``has_duplicate`` will receive a QuerySet of destinations and
+ a dict of settings for a possible destination and should return True if
+ a destination with such settings exists in the given QuerySet. By default it
+ will use MEDIA_SETTINGS_KEY to lookup the most important piece of
+ information in the settings.
+
+raise_if_not_deletable
+ ``raise_if_not_deletable`` should check if a given destination can be
+ deleted. This is necessary in case the destination is in use by a profile,
+ or some destinations are synced from an outside source or otherwise
+ auto-generated, and should not be able to be deleted by a user. If that is
+ the case a ``NotDeletableError`` should be raised. If not simply return
+ None.
+
+update
+ The method ``update`` only has to be implemented if the regular update
+ method is insufficient. This can be the case if there is more than one
+ key-value pair in settings that need to be updated.
+
+validate
+ The function ``validate`` makes sure that a destination with the given
+ settings can be updated or created. It uses the ``validate_settings`` method
+ to validate the settings-field, and a form (CommonDestinationConfigForm) to
+ validate the media and label-fields. The validated form is returned if ok,
+ otherwise a ``ValidationError`` should be raised. It is unlikely that you
+ will ever need to override this method.
+
+validate_settings
+ This method validates the actual contents of the settings-field using the
+ ``Form`` that is defined. The function ``has_duplicate`` can be used here to
+ ensure that not two destinations with the same settings will be created.
+ A ``ValidationError`` should be raised if the given settings are invalid,
+ and the validated and cleaned data should be returned if not.
diff --git a/src/argus/notificationprofile/media/base.py b/src/argus/notificationprofile/media/base.py
index 04c5dc1e1..738632f3b 100644
--- a/src/argus/notificationprofile/media/base.py
+++ b/src/argus/notificationprofile/media/base.py
@@ -3,18 +3,21 @@
from abc import ABC, abstractmethod
from typing import TYPE_CHECKING
+from django import forms
+from django.core.exceptions import ValidationError
+
+from ..models import DestinationConfig
+
if TYPE_CHECKING:
from collections.abc import Iterable
from types import NoneType
- from typing import Union
+ from typing import Union, Generator
from django.contrib.auth import get_user_model
from django.db.models.query import QuerySet
from argus.incident.models import Event
- from ..models import DestinationConfig
- from ..serializers import RequestDestinationConfigSerializer
User = get_user_model()
@@ -22,7 +25,39 @@
__all__ = ["NotificationMedium"]
+class CommonDestinationConfigForm(forms.ModelForm):
+ class Meta:
+ model = DestinationConfig
+ fields = ["media", "label"]
+
+
class NotificationMedium(ABC):
+ """
+ Must be defined by subclasses:
+
+ Class attributes:
+
+ - MEDIA_SLUG: short string id for the medium, lowercase
+ - MEDIA_NAME: human friendly id for the medium
+ - MEDIA_SETTINGS_KEY: the field in settings that is specific for this medium
+ - MEDIA_JSON_SCHEMA: A json-schema to describe the settings field to
+ javascript, used by the API
+ - Form: a django.forms.Form that validates the contents of the
+ settings-field. MEDIA_SETTINGS_KEY must be the field-name of a required
+ field in the form.
+
+ Class methods:
+
+ - send(event, destinations): How to send the given event to the given
+ destinations of type MEDIA_SLUG.
+ """
+
+ error_messages = {
+ "readonly_medium": "Media cannot be updated, only settings.",
+ "readonly_user": "User cannot be changed, only settings.",
+ "settings_type": "Settings has to be a dictionary.",
+ }
+
class NotDeletableError(Exception):
"""
Custom exception class that is raised when a destination cannot be
@@ -30,35 +65,80 @@ class NotDeletableError(Exception):
"""
@classmethod
- @abstractmethod
- def validate(cls, instance: RequestDestinationConfigSerializer, dict: dict, user: User) -> dict:
+ def validate(
+ cls, data: dict, user: User, instance: DestinationConfig = None, exception_class=ValidationError
+ ) -> CommonDestinationConfigForm:
+ if instance:
+ if data.get("media", "") != instance.media.slug:
+ raise exception_class(cls.error_messages["readonly_media"])
+ form = CommonDestinationConfigForm(data, instance=instance)
+ if instance.user != user:
+ raise exception_class(cls.error_messages["readonly_user"])
+ else:
+ form = CommonDestinationConfigForm(data)
+
+ if not form.is_valid():
+ raise exception_class(form.errors)
+
+ settings_form = cls.validate_settings(data, user)
+ form.cleaned_data["settings"] = settings_form.cleaned_data
+ form.cleaned_data["user"] = user
+ return form
+
+ @classmethod
+ def validate_settings(cls, data: dict, user: User, exception_class=ValidationError) -> dict:
"""
Validates the settings of destination and returns a dict with
validated and cleaned data
"""
- pass
+ form = cls.Form(data["settings"])
+ if not form.is_valid():
+ raise exception_class(form.errors)
+
+ form = cls.clean(form)
+
+ if cls.has_duplicate(user.destinations, data["settings"]):
+ raise exception_class(cls.MEDIA_SETTINGS_KEY, f"{cls.MEDIA_NAME} already exists")
+
+ return form.cleaned_data
+
+ @classmethod
+ def clean(cls, form: forms.Form) -> forms.Form:
+ """Can change the cleaned data of a valid form"""
+ return form
@classmethod
- @abstractmethod
def has_duplicate(cls, queryset: QuerySet, settings: dict) -> bool:
"""
Returns True if a destination with the given settings already exists
in the given queryset
"""
- pass
+ key = f"settings__{cls.MEDIA_SETTINGS_KEY}"
+ value = settings[cls.MEDIA_SETTINGS_KEY]
+ return queryset.filter(media_id=cls.MEDIA_SLUG, **{key: value}).exists()
- @staticmethod
- @abstractmethod
- def get_label(destination: DestinationConfig) -> str:
+ @classmethod
+ def get_label(cls, destination: DestinationConfig) -> str:
"""
- Returns a descriptive label for this destination.
+ Returns a descriptive label for this destination if none is stored
"""
- pass
+ return destination.label if destination.label else destination.settings.get(cls.MEDIA_SETTINGS_KEY)
+
+ # No querysets beyond this point!
+
+ @classmethod
+ def _get_relevant_destinations(cls, destinations: Iterable[DestinationConfig]) -> Generator[DestinationConfig]:
+ return (destination for destination in destinations if destination.media_id == cls.MEDIA_SLUG)
@classmethod
- def get_relevant_addresses(cls, destinations: Iterable[DestinationConfig]) -> set[DestinationConfig]:
+ def get_relevant_destination_settings(cls, destinations: Iterable[DestinationConfig]) -> set[str]:
"""Returns a set of addresses the message should be sent to"""
- pass
+ destinations = [
+ destination.settings[cls.MEDIA_SETTINGS_KEY] for destination in cls._get_relevant_destinations(destinations)
+ ]
+ return set(destinations)
+
+ get_relevant_addresses = get_relevant_destination_settings
@classmethod
@abstractmethod
@@ -79,11 +159,23 @@ def raise_if_not_deletable(cls, destination: DestinationConfig) -> NoneType:
f"Cannot delete this destination since it is in use in the notification profile(s): {profiles}."
)
- @staticmethod
- def update(destination: DestinationConfig, validated_data: dict) -> Union[DestinationConfig, NoneType]:
+ @classmethod
+ def _update_destination(cls, destination: DestinationConfig, validated_data: dict) -> DestinationConfig:
+ # adapted from rest_framework.serializers.ModelSerializer.update
+ # DestinationConfig MUST NOT have any m2m-relations so this is safe
+
+ for attr, value in validated_data.items():
+ setattr(destination, attr, value)
+
+ return destination
+
+ @classmethod
+ def update(cls, destination: DestinationConfig, validated_data: dict) -> Union[DestinationConfig, NoneType]:
"""
- Updates a destination in case the normal update function is not
- sufficient and returns the updated destination in that case,
- returns None otherwise
+ Updates a destination
+
+ Override in case the normal update function is not sufficient
"""
- return None
+ instance = cls._update_destination(destination, validated_data)
+ instance.save()
+ return instance
diff --git a/src/argus/notificationprofile/media/email.py b/src/argus/notificationprofile/media/email.py
index 37e17a765..bb676757d 100644
--- a/src/argus/notificationprofile/media/email.py
+++ b/src/argus/notificationprofile/media/email.py
@@ -7,30 +7,29 @@
from django.conf import settings
from django.core.mail import send_mail
from django.template.loader import render_to_string
-from rest_framework.exceptions import ValidationError
from argus.incident.models import Event
+from argus.util.datetime_utils import INFINITY, LOCAL_INFINITY
+
from .base import NotificationMedium
from ..models import DestinationConfig
-from argus.util.datetime_utils import INFINITY, LOCAL_INFINITY
if TYPE_CHECKING:
from collections.abc import Iterable
from types import NoneType
- from typing import Union
from django.contrib.auth import get_user_model
- from django.db.models.query import QuerySet
-
- from ..serializers import RequestDestinationConfigSerializer
User = get_user_model()
+
LOG = logging.getLogger(__name__)
__all__ = [
+ "modelinstance_to_dict",
"send_email_safely",
+ "BaseEmailNotification",
"EmailNotification",
]
@@ -45,7 +44,7 @@ def send_email_safely(function, additional_error=None, *args, **kwargs) -> int:
try:
result = function(*args, **kwargs)
return result
- except ConnectionRefusedError as e:
+ except ConnectionRefusedError:
EMAIL_HOST = getattr(settings, "EMAIL_HOST", None)
if not EMAIL_HOST:
LOG.error("Notification: Email: EMAIL_HOST not set, cannot send")
@@ -59,100 +58,26 @@ def send_email_safely(function, additional_error=None, *args, **kwargs) -> int:
# TODO: Store error as incident
-class EmailNotification(NotificationMedium):
+class BaseEmailNotification(NotificationMedium):
MEDIA_SLUG = "email"
MEDIA_NAME = "Email"
+ MEDIA_SETTINGS_KEY = "email_address"
MEDIA_JSON_SCHEMA = {
"title": "Email Settings",
"description": "Settings for a DestinationConfig using email.",
"type": "object",
- "required": ["email_address"],
- "properties": {"email_address": {"type": "string", "title": "Email address"}},
+ "required": [MEDIA_SETTINGS_KEY],
+ "properties": {
+ MEDIA_SETTINGS_KEY: {
+ "type": "string",
+ "title": "Email address",
+ },
+ },
}
class Form(forms.Form):
- synced = forms.BooleanField(disabled=True, required=False, initial=False)
email_address = forms.EmailField()
- @classmethod
- def validate(cls, instance: RequestDestinationConfigSerializer, email_dict: dict, user: User) -> dict:
- """
- Validates the settings of an email destination and returns a dict
- with validated and cleaned data
- """
- form = cls.Form(email_dict["settings"])
- if not form.is_valid():
- raise ValidationError(form.errors)
- if form.cleaned_data["email_address"] == instance.context["request"].user.email:
- raise ValidationError("This email address is already registered in another destination.")
- if user.destinations.filter(
- media_id="email", settings__email_address=form.cleaned_data["email_address"]
- ).exists():
- raise ValidationError({"email_address": "Email address already exists"})
-
- return form.cleaned_data
-
- @classmethod
- def raise_if_not_deletable(cls, destination: DestinationConfig) -> NoneType:
- """
- Raises a NotDeletableError if the given email destination is not able
- to be deleted (if it was defined by an outside source or is in use by
- any notification profiles)
- """
- super().raise_if_not_deletable(destination=destination)
-
- if destination.settings["synced"]:
- raise cls.NotDeletableError(
- "Cannot delete this email destination since it was defined by an outside source."
- )
-
- @staticmethod
- def update(destination: DestinationConfig, validated_data: dict) -> Union[DestinationConfig, NoneType]:
- """
- Updates the synced email destination by copying its contents to
- a new destination and updating the given destination with the given
- validated data and returning the updated destination
-
- This way the synced destination is not lost
- """
- if destination.settings["synced"]:
- new_synced_destination = DestinationConfig(
- user=destination.user,
- media_id=destination.media_id,
- settings=destination.settings,
- )
- destination.settings = validated_data["settings"]
- DestinationConfig.objects.bulk_update([destination], fields=["settings"])
- new_synced_destination.save()
- return destination
- return None
-
- @staticmethod
- def get_label(destination: DestinationConfig) -> str:
- """
- Returns the e-mail address represented by this destination
- """
- return destination.settings.get("email_address")
-
- @classmethod
- def has_duplicate(cls, queryset: QuerySet, settings: dict) -> bool:
- """
- Returns True if an email destination with the same email address
- already exists in the given queryset
- """
- return queryset.filter(settings__email_address=settings["email_address"]).exists()
-
- @classmethod
- def get_relevant_addresses(cls, destinations: Iterable[DestinationConfig]) -> set[DestinationConfig]:
- """Returns a list of email addresses the message should be sent to"""
- email_addresses = [
- destination.settings["email_address"]
- for destination in destinations
- if destination.media_id == cls.MEDIA_SLUG
- ]
-
- return set(email_addresses)
-
@staticmethod
def create_message_context(event: Event):
"""Creates the subject, message and html message for the email"""
@@ -183,7 +108,7 @@ def send(cls, event: Event, destinations: Iterable[DestinationConfig], **_) -> b
Returns False if no email destinations were given and
True if emails were sent
"""
- email_addresses = cls.get_relevant_addresses(destinations=destinations)
+ email_addresses = cls.get_relevant_destination_settings(destinations=destinations)
if not email_addresses:
return False
num_emails = len(email_addresses)
@@ -214,3 +139,64 @@ def send(cls, event: Event, destinations: Iterable[DestinationConfig], **_) -> b
)
LOG.debug("Email: Failed to send to:", " ".join(failed))
return True
+
+
+class EmailNotification(BaseEmailNotification):
+ class Form(forms.Form):
+ synced = forms.BooleanField(disabled=True, required=False, initial=False)
+ email_address = forms.EmailField()
+
+ @classmethod
+ def raise_if_not_deletable(cls, destination: DestinationConfig) -> NoneType:
+ """
+ Raises a NotDeletableError if the given email destination is not able
+ to be deleted (if it was defined by an outside source or is in use by
+ any notification profiles)
+ """
+ super().raise_if_not_deletable(destination=destination)
+
+ if destination.settings["synced"]:
+ raise cls.NotDeletableError(
+ "Cannot delete this email destination since it was defined by an outside source."
+ )
+
+ @classmethod
+ def _clone_if_changing_email_address(cls, destination: DestinationConfig, validated_data: dict):
+ if not destination.settings["synced"]:
+ return False
+
+ new_address = validated_data["settings"][cls.MEDIA_SETTINGS_KEY]
+ old_address = destination.settings[cls.MEDIA_SETTINGS_KEY]
+ if new_address == old_address: # address wasn't changed
+ return False
+
+ settings = {
+ "synced": True,
+ cls.MEDIA_SETTINGS_KEY: old_address,
+ }
+ DestinationConfig.objects.create(
+ user=destination.user,
+ media_id=destination.media_id,
+ settings=settings,
+ label=cls.get_label(destination),
+ )
+ LOG.info("Cloning synced email-address on update: %s", old_address)
+ return True
+
+ @classmethod
+ def update(cls, destination: DestinationConfig, validated_data: dict) -> DestinationConfig:
+ """
+ Preserves a synced email destination by cloning its contents to
+ a new destination and updating the given destination with the given
+ validated data and returning the updated destination
+
+ This way the synced destination is not lost
+ """
+ cls._clone_if_changing_email_address(destination, validated_data)
+
+ # We cannot use super() here since this is not an instance method
+ instance = cls._update_destination(destination, validated_data)
+ instance.settings["synced"] = False
+
+ instance.save()
+ return instance
diff --git a/src/argus/notificationprofile/media/sms_as_email.py b/src/argus/notificationprofile/media/sms_as_email.py
index 57a5c31f2..cdc7757ab 100644
--- a/src/argus/notificationprofile/media/sms_as_email.py
+++ b/src/argus/notificationprofile/media/sms_as_email.py
@@ -13,7 +13,6 @@
from django.conf import settings
from django.core.mail import send_mail
from phonenumber_field.formfields import PhoneNumberField
-from rest_framework.exceptions import ValidationError
from ...incident.models import Event
from .base import NotificationMedium
@@ -23,10 +22,8 @@
from collections.abc import Iterable
from django.contrib.auth import get_user_model
- from django.db.models.query import QuerySet
from ..models import DestinationConfig
- from ..serializers import RequestDestinationConfigSerializer
User = get_user_model()
@@ -36,65 +33,28 @@
class SMSNotification(NotificationMedium):
MEDIA_SLUG = "sms"
MEDIA_NAME = "SMS"
+ MEDIA_SETTINGS_KEY = "phone_number"
MEDIA_JSON_SCHEMA = {
"title": "SMS Settings",
"description": "Settings for a DestinationConfig using SMS.",
"type": "object",
- "required": ["phone_number"],
+ "required": [MEDIA_SETTINGS_KEY],
"properties": {
- "phone_number": {
+ MEDIA_SETTINGS_KEY: {
"type": "string",
"title": "Phone number",
"description": "The phone number is validated and the country code needs to be given.",
- }
+ },
},
}
- class PhoneNumberForm(forms.Form):
+ class Form(forms.Form):
phone_number = PhoneNumberField()
@classmethod
- def validate(cls, instance: RequestDestinationConfigSerializer, sms_dict: dict, user: User) -> dict:
- """
- Validates the settings of an SMS destination and returns a dict
- with validated and cleaned data
- """
- form = cls.PhoneNumberForm(sms_dict["settings"])
- if not form.is_valid():
- raise ValidationError(form.errors)
-
- form.cleaned_data["phone_number"] = form.cleaned_data["phone_number"].as_e164
-
- if user.destinations.filter(media_id="sms", settings__phone_number=form.cleaned_data["phone_number"]).exists():
- raise ValidationError({"phone_number": "Phone number already exists"})
-
- return form.cleaned_data
-
- @staticmethod
- def get_label(destination: DestinationConfig) -> str:
- """
- Returns the phone number represented by this SMS destination
- """
- return destination.settings.get("phone_number")
-
- @classmethod
- def has_duplicate(cls, queryset: QuerySet, settings: dict) -> bool:
- """
- Returns True if a sms destination with the same phone number
- already exists in the given queryset
- """
- return queryset.filter(settings__phone_number=settings["phone_number"]).exists()
-
- @classmethod
- def get_relevant_addresses(cls, destinations: Iterable[DestinationConfig]) -> set[DestinationConfig]:
- """Returns a list of phone numbers the message should be sent to"""
- phone_numbers = [
- destination.settings["phone_number"]
- for destination in destinations
- if destination.media_id == cls.MEDIA_SLUG
- ]
-
- return set(phone_numbers)
+ def clean(cls, form: Form) -> Form:
+ form.cleaned_data[cls.MEDIA_SETTINGS_KEY] = form.cleaned_data[cls.MEDIA_SETTINGS_KEY].as_e164
+ return form
@classmethod
def send(cls, event: Event, destinations: Iterable[DestinationConfig], **_) -> bool:
@@ -108,7 +68,7 @@ def send(cls, event: Event, destinations: Iterable[DestinationConfig], **_) -> b
LOG.error("SMS_GATEWAY_ADDRESS is not set, cannot dispatch SMS notifications using this plugin")
return
- phone_numbers = cls.get_relevant_addresses(destinations=destinations)
+ phone_numbers = cls.get_relevant_destination_settings(destinations=destinations)
if not phone_numbers:
return False
diff --git a/src/argus/notificationprofile/serializers.py b/src/argus/notificationprofile/serializers.py
index af465aed1..a31eb67e9 100644
--- a/src/argus/notificationprofile/serializers.py
+++ b/src/argus/notificationprofile/serializers.py
@@ -137,27 +137,34 @@ class Meta:
]
def validate(self, attrs: dict):
- if self.instance and "media" in attrs.keys() and not attrs["media"].slug == self.instance.media.slug:
- raise serializers.ValidationError("Media cannot be updated, only settings.")
- if "settings" in attrs.keys():
- if not isinstance(attrs["settings"], dict):
- raise serializers.ValidationError("Settings has to be a dictionary.")
- if self.instance:
- medium = api_safely_get_medium_object(self.instance.media.slug)
- else:
- medium = api_safely_get_medium_object(attrs["media"].slug)
- attrs["settings"] = medium.validate(self, attrs, self.context["request"].user)
+ settings = attrs.get("settings", None)
+ if not settings:
+ raise serializers.ValidationError("Settings cannot be empty")
+
+ if self.instance:
+ medium = api_safely_get_medium_object(self.instance.media.slug)
+ # A PATCH need not contain the media key
+ if attrs.get("media", None) != self.instance.media:
+ raise serializers.ValidationError(medium.error_messages["readonly_medium"])
+ user = self.instance.user
+ if user != self.context["request"].user:
+ raise serializers.ValidationError(medium.error_messages["readonly_user"])
+ else:
+ # new, not a PATCH
+ medium = api_safely_get_medium_object(attrs["media"].slug)
+ user = self.context["request"].user
+
+ if not isinstance(attrs["settings"], dict):
+ raise serializers.ValidationError(medium.error_messages["settings_type"])
+
+ attrs["settings"] = medium.validate_settings(attrs, user, serializers.ValidationError)
return attrs
def update(self, destination: DestinationConfig, validated_data: dict):
medium = api_safely_get_medium_object(destination.media.slug)
updated_destination = medium.update(destination, validated_data)
-
- if updated_destination:
- return updated_destination
-
- return super().update(destination, validated_data)
+ return updated_destination
class DuplicateDestinationSerializer(serializers.Serializer):