Repository: codingjoe/joeflow (language: python)

Issue:
Notes on first time working with the package
### 1) Workflow & State
For me the separation between a workflow and state is not quite obvious in the first place. 
Also, that a state is actually a new model is not obvious. (In the Tutorial it is not clear why the flow or the state needs a User)

### 2) Entry point of a Task
In the tutorial the start edge is defined as variable on the workflow. But it is unclear what all kinds of implications this has. Once you have reached all parts connected to the start edge it gets more clear.

### 3) Workflow creation
For me, it took some time to understand that for any workflow you want to create, you actually have to create an object. At first glance it could be like an add-on where you hook up to the model itself and just add tasks. But now I understand that each workflow needs a model representation (and that it can be an existing model). And that migrations are needed.

For a bit more complicated use cases or integration with an existing model it would be nice to have some documentation.

All in all it is a really nice package and well-thought-out. 


Discussion / hints:

Fix patch:
diff --git a/docs/index.rst b/docs/index.rst
index a13c4d3..db739bc 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -41,12 +41,14 @@ may look like:
     from joeflow import tasks
 
 
-    class Shipment(Workflow):
+    class ShippingWorkflow(Workflow):
+
+        # model fields to store the state
         email = models.EmailField(blank=True)
         shipping_address = models.TextField()
         tracking_code = models.TextField()
 
-    class ShippingWorkflow(Shipment):
+        # tasks (nodes)
         checkout = tasks.StartView(fields=["shipping_address", "email"])
 
         ship = tasks.UpdateView(fields=["tracking_code"])
@@ -66,6 +68,7 @@ may look like:
         def end(self):
             pass
 
+        # edges
         edges = [
             (checkout, ship),
             (ship, has_email),
@@ -74,9 +77,6 @@ may look like:
             (send_tracking_code, end),
         ]
 
-        class Meta:
-            proxy = True
-
 Design Principles
 =================
 
diff --git a/docs/tutorial/workflow.rst b/docs/tutorial/workflow.rst
index 52f1b01..4148903 100644
--- a/docs/tutorial/workflow.rst
+++ b/docs/tutorial/workflow.rst
@@ -32,27 +32,32 @@ store a user. Like so:
     from joeflow.models import Workflow
 
 
-    class WelcomeWorkflowState(Workflow):
+    class WelcomeWorkflow(Workflow):
+        # state
         user = models.ForeignKey(
             settings.AUTH_USER_MODEL,
             on_delete=models.CASCADE,
             blank=True, null=True,
         )
 
-
-We keep the model abstract. The abstract model will make it easier to separate
-state from behavior and therefore easier to read for your fellow developers.
-
 Next we add the behavior:
 
 .. code-block:: python
 
+    from django.conf import settings
     from joeflow import tasks
+    from joeflow.models import Workflow
 
-    from . import models
 
+    class WelcomeWorkflow(Workflow):
+        # state
+        user = models.ForeignKey(
+            settings.AUTH_USER_MODEL,
+            on_delete=models.CASCADE,
+            blank=True, null=True,
+        )
 
-    class WelcomeWorkflow(models.WelcomeWorkflowState):
+        # behavior
         start = tasks.StartView(fields=["user"])
 
         def has_user(self):
@@ -76,8 +81,6 @@ Next we add the behavior:
             (send_welcome_email, end),
         ]
 
-        class Meta:
-            proxy = True
 
 We have the tasks ``start``, ``has_user`` ``send_welcome_email`` and ``end``
 on the top and define all the edges on the bottom. Edges are defined by a
@@ -101,11 +104,11 @@ to your ``urls.py``:
 
     from django.urls import path, include
 
-    from . import workflows
+    from . import models
 
     urlpatterns = [
         # …
-        path('welcome/', include(workflows.WelcomeWorkflow.urls())),
+        path('welcome/', include(models.WelcomeWorkflow.urls())),
     ]
 
 This will add URLs for all human tasks as well as a detail view and manual


Test patch:
diff --git a/tests/test_docs.py b/tests/test_docs.py
index 983a217..e445334 100644
--- a/tests/test_docs.py
+++ b/tests/test_docs.py
@@ -3,7 +3,7 @@
 from django.test import SimpleTestCase, TestCase
 from django.urls import reverse
 
-from .testapp import workflows
+from .testapp import models
 
 
 class WelcomeWorkflowMachineTest(SimpleTestCase):
@@ -14,11 +14,11 @@ def test_has_user__with_user(self):
             last_name="Parker",
             username="spidy",
         )
-        workflow = workflows.WelcomeWorkflow(user=user)
+        workflow = models.WelcomeWorkflow(user=user)
         self.assertEqual(workflow.has_user(), [workflow.send_welcome_email])
 
     def test_has_user__without_user(self):
-        workflow = workflows.WelcomeWorkflow()
+        workflow = models.WelcomeWorkflow()
         self.assertEqual(workflow.has_user(), [workflow.end])
 
     def test_send_welcome_email(self):
@@ -28,7 +28,7 @@ def test_send_welcome_email(self):
             last_name="Parker",
             username="spidy",
         )
-        workflow = workflows.WelcomeWorkflow(user=user)
+        workflow = models.WelcomeWorkflow(user=user)
 
         workflow.send_welcome_email()
 
@@ -55,13 +55,13 @@ def test_start__post_with_user(self):
 
         response = self.client.post(self.start_url, data=dict(user=user.pk))
         self.assertEqual(response.status_code, 302)
-        workflow = workflows.WelcomeWorkflow.objects.get()
+        workflow = models.WelcomeWorkflow.objects.get()
         self.assertTrue(workflow.user)
         self.assertTrue(workflow.task_set.succeeded().filter(name="start").exists())
 
     def test_start__post_without_user(self):
         response = self.client.post(self.start_url)
         self.assertEqual(response.status_code, 302)
-        workflow = workflows.WelcomeWorkflow.objects.get()
+        workflow = models.WelcomeWorkflow.objects.get()
         self.assertFalse(workflow.user)
         self.assertTrue(workflow.task_set.succeeded().filter(name="start").exists())
diff --git a/tests/testapp/migrations/0001_initial.py b/tests/testapp/migrations/0001_initial.py
index f502582..f400b39 100644
--- a/tests/testapp/migrations/0001_initial.py
+++ b/tests/testapp/migrations/0001_initial.py
@@ -158,7 +158,7 @@ class Migration(migrations.Migration):
             bases=("joeflow.workflow",),
         ),
         migrations.CreateModel(
-            name="WelcomeWorkflowState",
+            name="WelcomeWorkflow",
             fields=[
                 (
                     "workflow_ptr",
@@ -231,10 +231,4 @@ class Migration(migrations.Migration):
             options={"proxy": True, "indexes": [], "constraints": []},
             bases=("testapp.waitworkflowstate",),
         ),
-        migrations.CreateModel(
-            name="WelcomeWorkflow",
-            fields=[],
-            options={"proxy": True, "indexes": [], "constraints": []},
-            bases=("testapp.welcomeworkflowstate",),
-        ),
     ]
diff --git a/tests/testapp/models.py b/tests/testapp/models.py
index 7cb1f20..399384d 100644
--- a/tests/testapp/models.py
+++ b/tests/testapp/models.py
@@ -1,6 +1,7 @@
 from django.conf import settings
 from django.db import models
 
+from joeflow import tasks
 from joeflow.models import Workflow
 
 
@@ -10,7 +11,8 @@ class Shipment(Workflow):
     tracking_code = models.TextField()
 
 
-class WelcomeWorkflowState(Workflow):
+class WelcomeWorkflow(Workflow):
+    # state
     user = models.ForeignKey(
         settings.AUTH_USER_MODEL,
         on_delete=models.CASCADE,
@@ -18,6 +20,31 @@ class WelcomeWorkflowState(Workflow):
         null=True,
     )
 
+    # behavior
+    start = tasks.StartView(fields=["user"])
+
+    def has_user(self):
+        if self.user:
+            return [self.send_welcome_email]
+        else:
+            return [self.end]
+
+    def send_welcome_email(self):
+        self.user.email_user(
+            subject="Welcome",
+            message="Hello %s!" % self.user.get_short_name(),
+        )
+
+    def end(self):
+        pass
+
+    edges = [
+        (start, has_user),
+        (has_user, end),
+        (has_user, send_welcome_email),
+        (send_welcome_email, end),
+    ]
+
 
 class SimpleWorkflowState(Workflow):
     pass
diff --git a/tests/testapp/urls.py b/tests/testapp/urls.py
index 57f10e7..cae03d2 100644
--- a/tests/testapp/urls.py
+++ b/tests/testapp/urls.py
@@ -16,7 +16,7 @@
 from django.contrib import admin
 from django.urls import include, path
 
-from . import workflows
+from . import models, workflows
 
 urlpatterns = [
     path("admin/", admin.site.urls),
@@ -26,5 +26,5 @@
     path("gateway/", include(workflows.GatewayWorkflow.urls())),
     path("splitjoin/", include(workflows.SplitJoinWorkflow.urls())),
     path("loop/", include(workflows.LoopWorkflow.urls())),
-    path("welcome/", include(workflows.WelcomeWorkflow.urls())),
+    path("welcome/", include(models.WelcomeWorkflow.urls())),
 ]
diff --git a/tests/testapp/workflows.py b/tests/testapp/workflows.py
index 4716e37..c0427c2 100644
--- a/tests/testapp/workflows.py
+++ b/tests/testapp/workflows.py
@@ -40,35 +40,6 @@ class Meta:
         proxy = True
 
 
-class WelcomeWorkflow(models.WelcomeWorkflowState):
-    start = tasks.StartView(fields=["user"])
-
-    def has_user(self):
-        if self.user:
-            return [self.send_welcome_email]
-        else:
-            return [self.end]
-
-    def send_welcome_email(self):
-        self.user.email_user(
-            subject="Welcome",
-            message="Hello %s!" % self.user.get_short_name(),
-        )
-
-    def end(self):
-        pass
-
-    edges = [
-        (start, has_user),
-        (has_user, end),
-        (has_user, send_welcome_email),
-        (send_welcome_email, end),
-    ]
-
-    class Meta:
-        proxy = True
-
-
 class SimpleWorkflow(models.SimpleWorkflowState):
     start_view = tasks.StartView(fields="__all__", path="custom/postfix/")
     start_method = tasks.Start()