Handle revoke model on SignoffField consistently.

signoffs/core/models/fields.py

@@ -100,7 +100,7 @@ def SignoffField(
     Convenience method for constructing from minimal inputs:
         (1) a sensible OneToOneField(signoff_type.signetModel); and
         (2) an RelatedSignoffDescriptor(signoff_type)
-    signoff_type may be an Approval Type or a registered(!) signoff id.
+    signoff_type may be a Signoff Type or a registered(!) signoff id.
     Default parameter rationale:
         null=True, on_delete=SET_NULL make sensible defaults for a Signet relation since presence/absence is semantic;
             think twice before using other values!
@@ -161,7 +161,7 @@ class SignoffSet:
     A descriptor that injects a "reverse relation" manager, filtered for the specific Signoff type.
     The signoff_type MUST be backed by a Signet with a FK relation to the owning model
     signet_set_accessor is a string with a path to the reverse relation manager on the owning model
-        - it may use '__' to travese relations and include method calls e.g.,  'my_signet_manager__get_signet_set'
+        - it may use '__' to traverse relations and include method calls e.g.,  'my_signet_manager__get_signet_set'
 
     In the example::
 

signoffs/core/signoffs.py

@@ -51,13 +51,14 @@ def revoke_signoff(signoff, user, reason="", revokeModel=None, **kwargs):
 
     @param revokeModel: if supplied, create record of revocation, otherwise just delete the signet.
     """
-    if revokeModel:
+    # always delete the signet to ensure any FK relations to signet are updated.
+    signoff.signet.delete()
+    signoff.signet.id = None
+    if revokeModel:   # restore the signet if we are keeping a record of its revocation.
+        signoff.signet.save()
         return revokeModel.objects.create(
             signet=signoff.signet, user=user, reason=reason
         )
-    else:
-        signoff.signet.delete()
-        signoff.signet.id = None
 
 
 class DefaultSignoffBusinessLogic:

signoffs/core/tests/models.py

@@ -147,6 +147,9 @@ class LeaveRequest(models.Model):
 
     # One-to-One "forward" relation - the OneToOneField is named employee_signet
     employee_signoff, employee_signet = SignoffField(employee_signoff_type)
+    # One-to-One "forward" relation with revoke receipts (note: this is not likely a very useful use-case)
+    revokable_signoff, revokable_signet = SignoffField(simple_revokable_signoff_type)
+
     # One-to-Many "reverse" relation based on relation defined by LeaveSignet
     hr_signoffs = SignoffSet(hr_signoff_type)
     mngmt_signoffs = SignoffSet(mngmt_signoff_type)

signoffs/core/tests/test_signoff_fields.py

@@ -53,6 +53,7 @@ def setUpTestData(cls):
         u3 = fixtures.get_user()
         lr = LeaveRequest.objects.create()
         lr.employee_signoff.sign_if_permitted(u1)
+        lr.revokable_signoff.sign_if_permitted(u1)
         cls.hr_signoffs = (
             lr.hr_signoffs.create(user=u1),
             lr.hr_signoffs.create(user=u2),
@@ -69,6 +70,26 @@ def test_signofffield(self):
             # OneToOne forward relation
             self.assertTrue(lr.employee_signoff.is_signed())
 
+    def test_signofffield_revoked(self):
+        """ Not sure what the use-case for this is, but it should do something sensible """
+        def get_leave_request():
+            return LeaveRequest.objects.select_related("revokable_signet__revoked", ).get(
+                pk=self.lr.pk
+            )
+        with self.assertNumQueries(1):
+            lr = get_leave_request()
+            self.assertTrue(lr.revokable_signoff.is_signed())
+        so = lr.revokable_signoff
+        so.revoke_if_permitted(user=self.u1)
+        # Revoke should leave the subject un-signed and with a NULL signet.
+        lr = get_leave_request()
+        self.assertFalse(lr.revokable_signoff.is_signed())
+        self.assertIsNone(lr.revokable_signet)
+        with self.assertNumQueries(1):
+            revoked = lr.revokable_signoff.get_revoked_signets_queryset().select_related('revoked__user')
+            self.assertEqual(len(revoked), 1)
+            self.assertTrue(all(s.revoked.user == self.u1 for s in revoked))
+
     def test_signoffset_manager(self):
         with self.assertNumQueries(2):
             lr = LeaveRequest.objects.prefetch_related("signatories").get(pk=self.lr.pk)