BREAKING CHANGE: SignoffForm.save() now returns the saved Signet inst…

…ead of its Signoff.

demo/article/views.py

@@ -149,9 +149,9 @@ def request_publication_view(request, article_id):
         request.POST
     )
     if signoff_form.is_valid() and signoff_form.is_signed_off():
-        signoff = signoff_form.sign(user=request.user, commit=False)
-        signoff.signet.article = article
-        signoff.save()
+        signet = signoff_form.sign(user=request.user, commit=False)
+        signet.article = article
+        signet.save()
         article.update_publication_status()
         article.save()
     return HttpResponseRedirect(reverse("article:detail", args=(article.id,)))
@@ -193,9 +193,9 @@ def approve_publication_view(request, article_id):
     )
 
     if signoff_form.is_valid() and signoff_form.is_signed_off():
-        signoff = signoff_form.sign(user=request.user, commit=False)
-        signoff.signet.article = article
-        signoff.save()
+        signet = signoff_form.sign(user=request.user, commit=False)
+        signet.article = article
+        signet.save()
     return HttpResponseRedirect(reverse("article:detail", args=(article.id,)))
 
 

signoffs/core/forms.py

@@ -80,7 +80,7 @@ def sign(self, user, commit=True):
         """
         Sign and save this form for the given user, without checking permissions (no business logic invoked!)
 
-        :return: the saved signoff instance, or None if the signoff was not actually signed.
+        :return: the saved Signet instance, or None if the signoff was not actually signed.
 
         :::{note}
         If signoff has m2m relations and commit==False, caller is responsible to call self.save_m2m()
@@ -90,7 +90,7 @@ def sign(self, user, commit=True):
         if self.is_signed_off() and signet:
             signoff = signet.signoff
             signoff.sign(user=user, commit=commit)
-            return signoff
+            return signoff.signet
         # nothing to do if it's not actually signed...
 
     def save(self, *args, **kwargs):

signoffs/core/tests/test_signoff_forms.py

@@ -51,8 +51,9 @@ def test_sign(self):
         bf = self.formClass(data=data)
         self.assertTrue(bf.is_valid())
         v = bf.sign(user=u, commit=False)
-        self.assertEqual(v.id, signoff_type.id)
-        self.assertEqual(v.signet.user, u)
+        self.assertIsInstance(v, models.AbstractSignet)
+        self.assertEqual(v.signoff_id, signoff_type.id)
+        self.assertEqual(v.user, u)
 
     def test_invalid_signoff(self):
         data = dict(signed_off="True", signoff_id="invalid.type")

signoffs/core/views/actions.py

@@ -174,9 +174,10 @@ def get_signoff_form(self):
     def get_signed_signoff(self, user):
         """Validate data against signoff form, return signed but unsaved signoff or None if form doesn't validate"""
         signoff_form = self.get_signoff_form()
-        if not signoff_form or not signoff_form.is_valid():
+        if not signoff_form or not signoff_form.is_signed_off():
             return None
-        signoff = signoff_form.sign(user=user, commit=False)
+        signet = signoff_form.sign(user=user, commit=False)
+        signoff = signet.signoff if signet else None
         if signoff and self.signoff_subject:
             signoff.subject = self.signoff_subject
         return signoff
@@ -259,10 +260,11 @@ def __init__(
         self.kwargs = kwargs
         self.signoff = None  # populated by calling sign_ or revoke_signoff
 
-    def verify_consistent_signet_id(self, signoff) -> bool:
-        """As an extra data integrity check, revoke URL's may also contain the signet_id - check it matches"""
-        signet_id = self.kwargs.get("signet_id", None)
-        return (signet_id == signoff.signet.pk) if signet_id else True
+    # NOT USED - moved to validator logic, see above  delete me
+    # def verify_consistent_signet_id(self, signoff) -> bool:
+    #     """As an extra data integrity check, revoke URL's may also contain the signet_id - check it matches"""
+    #     signet_id = self.kwargs.get("signet_id", None)
+    #     return (signet_id == signoff.signet.pk) if signet_id else True
 
     # "Template Method" hooks: to extend / override sign_signoff without duplicating core algorithm
 

tests/test_app/tests/test_model_fields.py

@@ -45,11 +45,11 @@ def test_signoff_field_form(self):
         )
         self.assertTrue(form.is_valid())
         self.assertTrue(form.is_signed_off())
-        v.employee_signoff = form.sign(user=self.employee, commit=True)
-        self.assertTrue(v.employee_signoff.is_signed())
-        self.assertEqual(v.employee_signoff.signatory, self.employee)
+        v.employee_signet = form.sign(user=self.employee, commit=True)
+        signoff = v.employee_signet.signoff
+        self.assertTrue(signoff.is_signed())
+        self.assertEqual(signoff.signatory, self.employee)
         # Must save Vacation instance to persist the FK relation!
-        v.employee_signet = v.employee_signoff.signet
         v.save()
         _, created = models.Vacation.objects.get_or_create(employee_signet__user=self.employee)
         self.assertFalse(created)
@@ -59,10 +59,7 @@ def test_signoff_field_form_signed(self):
         form = v.employee_signoff.forms.get_signoff_form(
             data={'signed_off':'on', 'signoff_id':'test_app.agree'}
         )
-        v.employee_signoff = form.sign(user=self.employee, commit=True)
-        self.assertTrue(v.employee_signoff.is_signed())
-        self.assertEqual(v.employee_signoff.signatory, self.employee)
-        v.employee_signet = v.employee_signoff.signet
+        v.employee_signet = form.sign(user=self.employee, commit=True)
         v.save()
 
         v, created = models.Vacation.objects.get_or_create(employee_signet__user=self.employee)

tests/test_app/tests/test_signoff_forms.py

@@ -24,10 +24,11 @@ def test_save(self):
         bf = self.get_form()
         self.assertTrue(bf.is_valid())
         v = bf.sign(user=u)
-        self.assertTrue(v.is_signed)
-        self.assertEqual(v.id, consent_signoff.id)
-        self.assertEqual(v.signet.user, u)
-        self.assertEqual(v.signet.sigil, u.get_full_name())
+        self.assertIsInstance(v, models.AbstractSignet)
+        self.assertTrue(v.signoff.is_signed())
+        self.assertEqual(v.signoff_id, consent_signoff.id)
+        self.assertEqual(v.user, u)
+        self.assertEqual(v.sigil, u.get_full_name())
 
     def test_invalid_save(self):
         bf = self.get_form()
@@ -59,9 +60,9 @@ def test_save_with_relation(self):
         self.assertTrue(bf.is_valid())
         v = bf.sign(user=u)
         signet = models.ReportSignet.objects.select_related("report").get(
-            pk=v.signet.pk
+            pk=v.pk
         )
-        self.assertEqual(v.signet.report, r)
+        self.assertEqual(v.report, r)
         report = models.Report.objects.prefetch_related("signatories").get(pk=r.pk)
         self.assertEqual(report.signatories.count(), 1)
         self.assertEqual(report.signatories.first(), signet)