Merge branch 'fix_max_notifications_warning' into 'master'

Fix: NOTIFICATIONS_MAX_PER_USER warning when not set

See merge request allianceauth/allianceauth!1383
This commit is contained in:
Ariel Rin 2021-12-24 04:53:40 +00:00
commit b80ee16a7c
2 changed files with 51 additions and 23 deletions

View File

@ -49,19 +49,22 @@ class NotificationManager(models.Manager):
logger.info("Created notification %s", obj)
return obj
def _max_notifications_per_user(self):
"""return the maximum number of notifications allowed per user"""
max_notifications = getattr(settings, 'NOTIFICATIONS_MAX_PER_USER', None)
if (
max_notifications is None
or not isinstance(max_notifications, int)
or max_notifications < 0
):
def _max_notifications_per_user(self) -> int:
"""Maximum number of notifications allowed per user."""
max_notifications = getattr(
settings,
"NOTIFICATIONS_MAX_PER_USER",
self.model.NOTIFICATIONS_MAX_PER_USER_DEFAULT
)
try:
max_notifications = int(max_notifications)
except ValueError:
max_notifications = None
if max_notifications is None or max_notifications < 0:
logger.warning(
'NOTIFICATIONS_MAX_PER_USER setting is invalid. Using default.'
"NOTIFICATIONS_MAX_PER_USER setting is invalid. Using default."
)
max_notifications = self.model.NOTIFICATIONS_MAX_PER_USER_DEFAULT
return max_notifications
def user_unread_count(self, user_pk: int) -> int:

View File

@ -1,5 +1,6 @@
from unittest.mock import patch
from django.conf import settings
from django.contrib.auth.models import User
from django.test import TestCase, override_settings
@ -113,29 +114,53 @@ class TestUserNotify(TestCase):
self.assertSetEqual(result, expected)
@patch("allianceauth.notifications.managers.logger")
@patch(
MODULE_PATH + '.Notification.NOTIFICATIONS_MAX_PER_USER_DEFAULT',
MODULE_PATH + ".Notification.NOTIFICATIONS_MAX_PER_USER_DEFAULT",
NOTIFICATIONS_MAX_PER_USER_DEFAULT
)
class TestMaxNotificationsPerUser(TestCase):
@override_settings(NOTIFICATIONS_MAX_PER_USER=None)
def test_reset_to_default_if_not_defined(self):
@override_settings(NOTIFICATIONS_MAX_PER_USER=42)
def test_should_use_custom_integer_setting(self, mock_logger):
# when
result = Notification.objects._max_notifications_per_user()
expected = NOTIFICATIONS_MAX_PER_USER_DEFAULT
self.assertEqual(result, expected)
# then
self.assertEqual(result, 42)
self.assertFalse(mock_logger.warning.called)
@override_settings(NOTIFICATIONS_MAX_PER_USER='11')
def test_reset_to_default_if_not_int(self):
@override_settings(NOTIFICATIONS_MAX_PER_USER="42")
def test_should_use_custom_string_setting(self, mock_logger):
# when
result = Notification.objects._max_notifications_per_user()
expected = NOTIFICATIONS_MAX_PER_USER_DEFAULT
self.assertEqual(result, expected)
# then
self.assertEqual(result, 42)
self.assertFalse(mock_logger.warning.called)
@override_settings()
def test_should_use_default_if_not_defined(self, mock_logger):
# given
del settings.NOTIFICATIONS_MAX_PER_USER
# when
result = Notification.objects._max_notifications_per_user()
# then
self.assertEqual(result, NOTIFICATIONS_MAX_PER_USER_DEFAULT)
self.assertFalse(mock_logger.warning.called)
@override_settings(NOTIFICATIONS_MAX_PER_USER="abc")
def test_should_reset_to_default_if_not_int(self, mock_logger):
# when
result = Notification.objects._max_notifications_per_user()
# then
self.assertEqual(result, NOTIFICATIONS_MAX_PER_USER_DEFAULT)
self.assertTrue(mock_logger.warning.called)
@override_settings(NOTIFICATIONS_MAX_PER_USER=-1)
def test_reset_to_default_if_lt_zero(self):
def test_should_reset_to_default_if_lt_zero(self, mock_logger):
# when
result = Notification.objects._max_notifications_per_user()
expected = NOTIFICATIONS_MAX_PER_USER_DEFAULT
self.assertEqual(result, expected)
# then
self.assertEqual(result, NOTIFICATIONS_MAX_PER_USER_DEFAULT)
self.assertTrue(mock_logger.warning.called)
@patch('allianceauth.notifications.managers.cache')