Merge pull request #140 from benjaoming/settings-save-fix

Test the correctness of Settings.save()

django_nyt/models.py

@@ -206,13 +206,13 @@ def save(self, *args, **kwargs):
         # it's not possible to create a database constraint for this.
         # Instead of having a constraint, we unset all other is_default for
         # the user.
+        default_settings = Settings.objects.filter(
+            user=self.user,
+            is_default=True,
+        ).exclude(pk=self.pk)
         if self.is_default:
-            default_settings = Settings.objects.filter(
-                user=self.user,
-                is_default=True,
-            ).exclude(pk=self.pk)
             default_settings.update(is_default=False)
-        else:
+        elif not default_settings.exists():
             non_default_settings = Settings.objects.filter(
                 user=self.user,
                 is_default=False,

docs/release_notes.rst

@@ -33,6 +33,7 @@ Release Notes
 * Template files possible for email subjects. Previously, this file was ignored #125 (Benjamin Balder Bach)
 * Notifications without URLs had a broken URL in emails #125 (Benjamin Balder Bach)
 * Management command ``notifymail`` to send emails is more robust #129 (Benjamin Balder Bach)
+* ``Settings.save`` recursively called itself when adding a non-default setting #140 (Benjamin Balder Bach)
 
 **Removed**
 

tests/core/test_basic.py

@@ -4,7 +4,9 @@
 
 from django_nyt import models
 from django_nyt import utils
+from django_nyt.conf import WEEKLY
 from django_nyt.decorators import disable_notify
+from django_nyt.models import Settings
 from tests.testapp.models import TestModel
 
 User = get_user_model()
@@ -142,3 +144,16 @@ def test_unsubscribe(self):
         # Check that no notification is generated here
         utils.notify("Test related object", self.TEST_KEY)
         assert models.Notification.objects.all().count() == notifications_before + 1
+
+
+class NotifySettingsTest(NotifyTestBase):
+    def test_create_settings(self):
+        # Create another user setting
+
+        Settings.objects.create(user=self.user1, interval=WEEKLY, is_default=False)
+        assert Settings.objects.filter(user=self.user1, is_default=True).count() == 1
+        assert Settings.objects.filter(user=self.user1).count() == 2
+
+        Settings.objects.create(user=self.user1, interval=WEEKLY, is_default=True)
+        assert Settings.objects.filter(user=self.user1, is_default=True).count() == 1
+        assert Settings.objects.filter(user=self.user1).count() == 3