Merge pull request #8 from garyd203/dont-call-hook-twice

Don't call a lifecycle hook twice with the same state.

.gitignore

@@ -12,5 +12,5 @@ README.txt
 .tox
 django_lifecycle.egg-info
 site/
+.idea
 venv
-.idea

django_lifecycle/mixins.py

@@ -203,17 +203,19 @@ def _run_hooked_methods(self, hook: str) -> List[str]:
                 when_any_field = callback_specs.get("when_any")
 
                 if when_field:
-                    if self._check_callback_conditions(when_field, callback_specs):
-                        fired.append(method.__name__)
-                        method(self)
+                    if not self._check_callback_conditions(when_field, callback_specs):
+                        continue
                 elif when_any_field:
-                    for field_name in when_any_field:
-                        if self._check_callback_conditions(field_name, callback_specs):
-                            fired.append(method.__name__)
-                            method(self)
-                else:
-                    fired.append(method.__name__)
-                    method(self)
+                    if not any([
+                        self._check_callback_conditions(field_name, callback_specs)
+                        for field_name in when_any_field
+                    ]):
+                        continue
+
+                # Only call the method once per hook
+                fired.append(method.__name__)
+                method(self)
+                break
 
         return fired
 

tests/testapp/migrations/0003_useraccount_name_changes.py

@@ -0,0 +1,18 @@
+# Generated by Django 2.2.8 on 2020-02-03 10:45
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('testapp', '0002_auto_20190928_2324'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='useraccount',
+            name='name_changes',
+            field=models.IntegerField(default=0),
+        ),
+    ]

tests/testapp/models.py

@@ -32,6 +32,7 @@ class UserAccount(LifecycleModel):
     joined_at = models.DateTimeField(null=True)
     has_trial = models.BooleanField(default=False)
     organization = models.ForeignKey(Organization, null=True, on_delete=models.SET_NULL)
+    name_changes = models.IntegerField(default=0)
 
     status = models.CharField(
         default="active",
@@ -61,7 +62,12 @@ def do_after_create_jobs(self):
     def timestamp_password_change(self):
         self.password_updated_at = timezone.now()
 
-    @hook("before_delete", when="has_trial", was="*", is_now=True)
+    @hook('before_update', when='first_name', has_changed=True)
+    @hook('before_update', when='last_name', has_changed=True)
+    def count_name_changes(self):
+        self.name_changes += 1
+
+    @hook("before_delete", when='has_trial', was='*', is_now=True)
     def ensure_trial_not_active(self):
         raise CannotDeleteActiveTrial("Cannot delete trial user!")
 

tests/testapp/tests/test_mixin.py

@@ -42,6 +42,7 @@ def test_snapshot_state(self):
                 "organization_id": org.id,
                 "status": "active",
                 "organization.name": "Dunder Mifflin",
+                "name_changes": 0,
             },
         )
 

tests/testapp/tests/test_user_account.py

@@ -56,6 +56,13 @@ def test_email_after_delete(self):
         self.assertEqual(len(mail.outbox), 1)
         self.assertEqual(mail.outbox[0].subject, "We have deleted your account")
 
+    def test_only_call_hook_once(self):
+        account = UserAccount.objects.create(**self.stub_data)
+        account.first_name = "Waylon"
+        account.last_name = "Smithers"
+        account.save()
+        self.assertEqual(account.name_changes, 1)
+
     def test_lowercase_email(self):
         data = self.stub_data
         data["email"] = "[email protected]"