diff --git a/demo/article/views.py b/demo/article/views.py index aab0913..aa3b77f 100644 --- a/demo/article/views.py +++ b/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,))) diff --git a/signoffs/core/forms.py b/signoffs/core/forms.py index cae5c9c..ef1db00 100644 --- a/signoffs/core/forms.py +++ b/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): diff --git a/signoffs/core/tests/test_signoff_forms.py b/signoffs/core/tests/test_signoff_forms.py index c9d0a1a..b54f765 100644 --- a/signoffs/core/tests/test_signoff_forms.py +++ b/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") diff --git a/signoffs/core/views/actions.py b/signoffs/core/views/actions.py index e36554e..3ce9ddd 100644 --- a/signoffs/core/views/actions.py +++ b/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 diff --git a/tests/test_app/tests/test_model_fields.py b/tests/test_app/tests/test_model_fields.py index da89ea5..c0dd0d9 100644 --- a/tests/test_app/tests/test_model_fields.py +++ b/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) diff --git a/tests/test_app/tests/test_signoff_forms.py b/tests/test_app/tests/test_signoff_forms.py index ef91fa5..43ce847 100644 --- a/tests/test_app/tests/test_signoff_forms.py +++ b/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)