From 6022246bf83a8a23978e4dab5ec5ede0f2b09b29 Mon Sep 17 00:00:00 2001 From: Daniel Perrefort Date: Mon, 12 Aug 2024 10:48:31 -0400 Subject: [PATCH] Fixes bug in set_user_preference where existing preferences cannot be updated (#385) --- keystone_api/apps/notifications/admin.py | 1 + .../notifications/migrations/0001_initial.py | 4 +- keystone_api/apps/notifications/models.py | 24 +++++-- .../notifications/tests/models/__init__.py | 0 .../tests/models/test_Preference.py | 65 +++++++++++++++++++ 5 files changed, 86 insertions(+), 8 deletions(-) create mode 100644 keystone_api/apps/notifications/tests/models/__init__.py create mode 100644 keystone_api/apps/notifications/tests/models/test_Preference.py diff --git a/keystone_api/apps/notifications/admin.py b/keystone_api/apps/notifications/admin.py index 6c2a0981..4809f95a 100644 --- a/keystone_api/apps/notifications/admin.py +++ b/keystone_api/apps/notifications/admin.py @@ -4,6 +4,7 @@ interfaces for managing application database constructs. """ +from django.conf import settings from django.contrib import admin from .models import * diff --git a/keystone_api/apps/notifications/migrations/0001_initial.py b/keystone_api/apps/notifications/migrations/0001_initial.py index 1860af0d..d8111c9e 100644 --- a/keystone_api/apps/notifications/migrations/0001_initial.py +++ b/keystone_api/apps/notifications/migrations/0001_initial.py @@ -31,9 +31,9 @@ class Migration(migrations.Migration): name='Preference', fields=[ ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('alloc_thresholds', models.JSONField(default=apps.notifications.models._default_alloc_thresholds)), + ('alloc_thresholds', models.JSONField(default=apps.notifications.models.default_alloc_thresholds)), ('notify_status_update', models.BooleanField(default=True)), - ('expiry_thresholds', models.JSONField(default=apps.notifications.models._default_expiry_thresholds)), + ('expiry_thresholds', models.JSONField(default=apps.notifications.models.default_expiry_thresholds)), ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), ], ), diff --git a/keystone_api/apps/notifications/models.py b/keystone_api/apps/notifications/models.py index 68b0709f..7b0bf4e0 100644 --- a/keystone_api/apps/notifications/models.py +++ b/keystone_api/apps/notifications/models.py @@ -11,12 +11,24 @@ from django.conf import settings from django.db import models +__all__ = ['Notification', 'Preference'] + + +def default_alloc_thresholds() -> list[int]: # pragma: nocover + """The default allocating usage thresholds at which to issue a user notification. + + Returned values are defined in units of percent usage. + """ -def _default_alloc_thresholds() -> list[int]: return [90] -def _default_expiry_thresholds() -> list[int]: +def default_expiry_thresholds() -> list[int]: # pragma: nocover + """The default expiration thresholds at which to issue a user notification. + + Returned values are defined in units of days until expiration. + """ + return [30, 14, 0] @@ -47,9 +59,9 @@ class NotificationType(models.TextChoices): class Preference(models.Model): """User notification preferences.""" - alloc_thresholds = models.JSONField(default=_default_alloc_thresholds) + alloc_thresholds = models.JSONField(default=default_alloc_thresholds) notify_status_update = models.BooleanField(default=True) - expiry_thresholds = models.JSONField(default=_default_expiry_thresholds) + expiry_thresholds = models.JSONField(default=default_expiry_thresholds) user = models.OneToOneField(settings.AUTH_USER_MODEL, on_delete=models.CASCADE) @@ -61,7 +73,7 @@ def get_user_preference(cls, user: settings.AUTH_USER_MODEL) -> Preference: return preference @classmethod - def set_user_preference(cls, *args, **kwargs) -> None: + def set_user_preference(cls, user: settings.AUTH_USER_MODEL, **kwargs) -> None: """Set user preferences, creating or updating as necessary.""" - cls.objects.create(*args, **kwargs) + cls.objects.update_or_create(user=user, defaults=kwargs) diff --git a/keystone_api/apps/notifications/tests/models/__init__.py b/keystone_api/apps/notifications/tests/models/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/keystone_api/apps/notifications/tests/models/test_Preference.py b/keystone_api/apps/notifications/tests/models/test_Preference.py new file mode 100644 index 00000000..136fd861 --- /dev/null +++ b/keystone_api/apps/notifications/tests/models/test_Preference.py @@ -0,0 +1,65 @@ +"""Unit tests for the `Preference` class.""" + +from django.contrib.auth import get_user_model +from django.test import TestCase + +from apps.notifications.models import default_alloc_thresholds, default_expiry_thresholds, Preference + +User = get_user_model() + + +class PreferenceModelTest(TestCase): + """Tests for getting user preferences.""" + + def setUp(self) -> None: + """Create a test user.""" + + self.user = User.objects.create_user(username='testuser', password='foobar123!') + + def test_get_user_preference_creates_new_preference(self) -> None: + """Test a new Preference object is created if one does not exist.""" + + # Test a record is created + self.assertFalse(Preference.objects.filter(user=self.user).exists()) + preference = Preference.get_user_preference(user=self.user) + self.assertTrue(Preference.objects.filter(user=self.user).exists()) + + # Ensure preference is created with appropriate defaults + self.assertEqual(self.user, preference.user) + self.assertListEqual(default_alloc_thresholds(), preference.alloc_thresholds) + self.assertListEqual(default_expiry_thresholds(), preference.expiry_thresholds) + + def test_get_user_preference_returns_existing_preference(self) -> None: + """Test an existing Preference object is returned if it already exists.""" + + existing_preference = Preference.objects.create(user=self.user) + preference = Preference.get_user_preference(user=self.user) + self.assertEqual(existing_preference, preference) + + +class PreferenceSetTest(TestCase): + """Tests for setting user preferences.""" + + def setUp(self) -> None: + """Create a test user.""" + + self.user = User.objects.create_user(username='testuser', password='foobar123!') + + def test_set_user_preference_creates_preference(self) -> None: + """Test that a new Preference object is created with specified values.""" + + self.assertFalse(Preference.objects.filter(user=self.user).exists()) + + Preference.set_user_preference(user=self.user, notify_status_update=False) + preference = Preference.objects.get(user=self.user) + self.assertFalse(preference.notify_status_update) + + def test_set_user_preference_updates_existing_preference(self) -> None: + """Test that an existing Preference object is updated with specified values.""" + + preference = Preference.objects.create(user=self.user, notify_status_update=True) + self.assertTrue(Preference.objects.filter(user=self.user).exists()) + + Preference.set_user_preference(user=self.user, notify_status_update=False) + preference.refresh_from_db() + self.assertFalse(preference.notify_status_update)