From 4feeab1376eccb9d4e6d167e025650aeba97cf0f Mon Sep 17 00:00:00 2001 From: hoang Date: Mon, 28 Sep 2026 11:07:41 +0700 Subject: [PATCH 1/2] Fix bug that email notifier's value is not validated through APIs We overlooked validating the email notifier's value during creation or updates via APIs. To fully resolve this issue, we have added validation at the model layer. As a result, the existing similar validation in admin.py is removed. --- promgen/admin.py | 7 ------- promgen/models.py | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/promgen/admin.py b/promgen/admin.py index fde2f0bbc..06f20bd9a 100644 --- a/promgen/admin.py +++ b/promgen/admin.py @@ -7,14 +7,12 @@ from django.contrib import admin from django.contrib.auth.admin import UserAdmin from django.contrib.auth.models import User -from django.core import validators from django.urls import path from django.utils.html import format_html from knox.admin import AuthTokenAdmin from promgen import actions, models, plugins, views from promgen.forms import PromgenAuthTokenCreateForm -from promgen.notification.email import NotificationEmail class PrometheusInline(admin.TabularInline): @@ -65,11 +63,6 @@ class Meta: model = models.Sender exclude = ["content_object"] - def clean(self): - if self.cleaned_data["sender"] == NotificationEmail.__module__: - validator = validators.EmailValidator() - validator(self.cleaned_data["value"]) - @admin.register(models.Sender) class SenderAdmin(admin.ModelAdmin): diff --git a/promgen/models.py b/promgen/models.py index 35160710c..aa1172207 100644 --- a/promgen/models.py +++ b/promgen/models.py @@ -11,6 +11,7 @@ from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation from django.contrib.contenttypes.models import ContentType from django.core.exceptions import FieldDoesNotExist +from django.core.validators import EmailValidator from django.db import models, transaction from django.forms.models import model_to_dict from django.urls import reverse @@ -178,6 +179,20 @@ def filtered(self, alert): def get_absolute_url(self): return reverse("notifier-edit", kwargs={"pk": self.pk}) + def clean(self): + super().clean() + if self.sender == "promgen.notification.email": + validator = EmailValidator() + validator(self.value) + + def save(self, *args, **kwargs): + # Django model validation is not automatically invoked when calling the original method + # save(). It runs automatically when data is handled through standard Django Forms or + # ModelForms. Therefore, we need to explicitly call full_clean() before saving the model + # instance to ensure that all field validations are always performed. + self.full_clean() + super().save(*args, **kwargs) + class Filter(models.Model): sender = models.ForeignKey("Sender", on_delete=models.CASCADE) From 23f03437c5a7aa22036e8735af794f98b6f143ca Mon Sep 17 00:00:00 2001 From: hoang Date: Mon, 28 Sep 2026 11:36:28 +0700 Subject: [PATCH 2/2] Return Django ValidationError messages in DRF's responses Previously, when a DjangoValidationError occurred, DRF would return a generic 500 Server Error response, which is not user-friendly. This kind of error should be shown in detail to user. Therefore, we have updated the DRF custom_exception_handler to convert DjangoValidationError into DRFValidationError. This change ensures that detailed messages are generated and returned to the user. --- promgen/rest_v2.py | 5 +++++ promgen/tests/test_rest.py | 11 ----------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/promgen/rest_v2.py b/promgen/rest_v2.py index cdf8b3e4f..d764e51b0 100644 --- a/promgen/rest_v2.py +++ b/promgen/rest_v2.py @@ -23,6 +23,7 @@ from rest_framework.exceptions import MethodNotAllowed, NotFound, PermissionDenied, ValidationError from rest_framework.renderers import TemplateHTMLRenderer from rest_framework.response import Response +from rest_framework.serializers import as_serializer_error from rest_framework.views import APIView, exception_handler import promgen.templatetags.promgen as promgen_templatetags @@ -78,6 +79,10 @@ def get_urls(self): def custom_exception_handler(exc, context): + if isinstance(exc, DjangoValidationError): + # Convert DjangoValidationError to a DRFValidationError + exc = ValidationError(detail=as_serializer_error(exc)) + # Call REST framework's default exception handler first, # to get the standard error response. response = exception_handler(exc, context) diff --git a/promgen/tests/test_rest.py b/promgen/tests/test_rest.py index aa5f5b6a3..cf39b2e92 100644 --- a/promgen/tests/test_rest.py +++ b/promgen/tests/test_rest.py @@ -662,14 +662,3 @@ def test_exception_handler(self): ) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), {"name": ["This field may not be blank."]}) - - # Unexpected exception (django.ValidationError) returns 500 HTTP code with - # the default response. - response = self.client.patch( - reverse("api-v2:rule-detail", kwargs={"id": 1}), - data={"clause": "This clause is not valid."}, - content_type="application/json", - HTTP_AUTHORIZATION=f"Token {admin_token}", - ) - self.assertEqual(response.status_code, 500) - self.assertEqual(response.json(), {"error": "Server Error (500)"})