State changes on exception. Merge @bacongobbler idea. Close #39

Author: kmmbvnr

README.md

@@ -121,6 +121,8 @@ model = BlogPost()
 model.state = 'invalid' # Raises AttributeError
 ```
 
+
+### `custom` properties
 Custom properties can be added by providing a dictionary to the `custom` keyword on the `transition` decorator.
 ```python
 @transition(field=state,
@@ -133,6 +135,18 @@ def legal_hold(self):
     """
 ```
 
+### `on_error` state
+
+In case of transition method would raise exception, you can provide specific target state
+
+```python
+@transition(field=state, source='new', target='published', on_error='failed')
+def publish(self):
+   """
+   Some exceptio could happends here
+   """
+```
+
 ### `state_choices`
 Instead of passing two elements list `choices` you could use three elements `state_choices`,
 the last element states for string reference to model proxy class.
@@ -295,7 +309,7 @@ that have been executed in an inconsistent (out of sync) state, thus practically
 
 Renders a graphical overview of your models states transitions
 
-You need `pip install graphviz` library
+You need `pip install graphviz>=0.4` library
 
 ```bash
 # Create a dot file
@@ -314,6 +328,8 @@ Changelog
 * Support for [class substitution](http://schinckel.net/2013/06/13/django-proxy-model-state-machine/) to proxy classes depending on the state
 * Added ConcurrentTransitionMixin with optimistic locking support
 * Default db_index=True for FSMIntegerField removed
+* Graph transition code migrated to new graphviz library with python 3 support
+* Ability to change state on transition exception
 
 ### django-fsm 2.1.0 2014-05-15
 * Support for attaching permission checks on model transitions

django_fsm/__init__.py

@@ -60,10 +60,11 @@ class ConcurrentTransition(Exception):
 
 
 class Transition(object):
-    def __init__(self, method, source, target, conditions, permission, custom):
+    def __init__(self, method, source, target, on_error, conditions, permission, custom):
         self.method = method
         self.source = source
         self.target = target
+        self.on_error = on_error
         self.conditions = conditions
         self.permission = permission
         self.custom = custom
@@ -132,14 +133,15 @@ def get_transition(self, source):
             transition = self.transitions.get('*', None)
         return transition
 
-    def add_transition(self, method, source, target, conditions=[], permission=None, custom={}):
+    def add_transition(self, method, source, target, on_error=None, conditions=[], permission=None, custom={}):
         if source in self.transitions:
             raise AssertionError('Duplicate transition for {0} state'.format(source))
 
         self.transitions[source] = Transition(
             method=method,
             source=source,
             target=target,
+            on_error=on_error,
             conditions=conditions,
             permission=permission,
             custom=custom)
@@ -179,6 +181,14 @@ def next_state(self, current_state):
 
         return transition.target
 
+    def exception_state(self, current_state):
+        transition = self.get_transition(current_state)
+
+        if transition is None:
+            raise TransitionNotAllowed('No transition from {0}'.format(current_state))
+
+        return transition.on_error
+
 
 class FSMFieldDescriptor(object):
     def __init__(self, field):
@@ -273,12 +283,21 @@ def change_state(self, instance, method, *args, **kwargs):
 
         pre_transition.send(**signal_kwargs)
 
-        result = method(instance, *args, **kwargs)
-        if next_state:
-            self.set_proxy(instance, next_state)
-            self.set_state(instance, next_state)
-
-        post_transition.send(**signal_kwargs)
+        try:
+            result = method(instance, *args, **kwargs)
+            if next_state:
+                self.set_proxy(instance, next_state)
+                self.set_state(instance, next_state)
+        except Exception as exc:
+            exception_state = meta.exception_state(current_state)
+            if exception_state:
+                self.set_proxy(instance, exception_state)
+                self.set_state(instance, exception_state)
+                signal_kwargs['target'] = exception_state
+                signal_kwargs['exception'] = exc
+            raise
+        finally:
+            post_transition.send(**signal_kwargs)
 
         return result
 
@@ -431,7 +450,7 @@ def save(self, *args, **kwargs):
         self._update_initial_state()
 
 
-def transition(field, source='*', target=None, conditions=[], permission=None, custom={}):
+def transition(field, source='*', target=None, on_error=None, conditions=[], permission=None, custom={}):
     """
     Method decorator for mark allowed transitions
 
@@ -450,9 +469,9 @@ def _change_state(instance, *args, **kwargs):
 
         if isinstance(source, (list, tuple)):
             for state in source:
-                func._django_fsm.add_transition(func, state, target, conditions, permission, custom)
+                func._django_fsm.add_transition(func, state, target, on_error, conditions, permission, custom)
         else:
-            func._django_fsm.add_transition(func, source, target, conditions, permission, custom)
+            func._django_fsm.add_transition(func, source, target, on_error, conditions, permission, custom)
 
         return _change_state
 

django_fsm/signals.py

@@ -2,4 +2,4 @@
 from django.dispatch import Signal
 
 pre_transition = Signal(providing_args=['instance', 'name', 'source', 'target'])
-post_transition = Signal(providing_args=['instance', 'name', 'source', 'target'])
+post_transition = Signal(providing_args=['instance', 'name', 'source', 'target', 'exception'])

tests/testapp/tests/test_exception_transitions.py

@@ -0,0 +1,39 @@
+from django.db import models
+from django.test import TestCase
+
+from django_fsm import FSMField, transition, can_proceed
+from django_fsm.signals import post_transition
+
+
+class ExceptionalBlogPost(models.Model):
+    state = FSMField(default='new')
+
+    @transition(field=state, source='new', target='published', on_error='crashed')
+    def publish(self):
+        raise Exception('Upss')
+
+    @transition(field=state, source='new', target='deleted')
+    def delete(self):
+        raise Exception('Upss')
+
+
+class FSMFieldExceptionTest(TestCase):
+    def setUp(self):
+        self.model = ExceptionalBlogPost()
+        post_transition.connect(self.on_post_transition, sender=ExceptionalBlogPost)
+        self.post_transition_data = None
+
+    def on_post_transition(self, **kwargs):
+        self.post_transition_data = kwargs
+
+    def test_state_changed_after_fail(self):
+        self.assertTrue(can_proceed(self.model.publish))
+        self.assertRaises(Exception, self.model.publish)
+        self.assertEqual(self.model.state, 'crashed')
+        self.assertEqual(self.post_transition_data['target'], 'crashed')
+        self.assertTrue('exception' in self.post_transition_data)
+
+    def test_state_not_changed_after_fail(self):
+        self.assertTrue(can_proceed(self.model.delete))
+        self.assertRaises(Exception, self.model.delete)
+        self.assertEqual(self.model.state, 'new')

tox.ini

@@ -5,22 +5,28 @@ envlist = py26, py27, py33
 usedevelop = True
 commands = python tests/manage.py {posargs:jenkins --pep8-max-line-length=150 --output-dir=reports/{envname}}
 deps = -r{toxinidir}/requirements.txt
+       graphviz>=0.4
        django-jenkins
        coverage
        pep8
        pyflakes
-       graphviz
        ipdb
 
 
 [testenv:py26]
-deps = {[testenv]deps}
+deps = django==1.6.5
        ipython==2.1.0
+       graphviz>=0.4
+       django-jenkins
+       coverage
+       pep8
+       pyflakes
+       ipdb
 
 [testenv:alpha]
 basepython = python3.3
 deps = git+https://github.com/django/django.git
-       graphviz
+       graphviz>=0.4
        django-jenkins
        coverage
        pep8