Merge pull request #37 from thinkt4nk/release_3.4.1

Release 3.4.1

issue/migrations/0005_responder_allow_retry.py

@@ -0,0 +1,38 @@
+# Generated by Django 2.2.4 on 2019-08-20 15:45
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+from . import datamigrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('contenttypes', '0002_remove_content_type_name'),
+        ('issue', '0004_auto_20181210_1857'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='responder',
+            name='allow_retry',
+            field=models.BooleanField(default=False),
+        ),
+        migrations.AddField(
+            model_name='issueaction',
+            name='action_issue_id',
+            field=models.PositiveIntegerField(null=True),
+        ),
+        migrations.AddField(
+            model_name='issueaction',
+            name='action_issue_type',
+            field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='+', to='contenttypes.ContentType'),
+        ),
+        migrations.RunPython(datamigrations.normalize_generic_issues),
+        migrations.AlterField(
+            model_name='issueaction',
+            name='issue',
+            field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='+', to='issue.Issue'),
+        ),
+    ]

issue/migrations/datamigrations/__init__.py

@@ -0,0 +1,13 @@
+from django.contrib.contenttypes.models import ContentType
+
+from issue.models import IssueAction, Issue
+
+
+def normalize_generic_issues(apps, schema_editor):
+    """
+    Normalize related issues as generic reference
+    """
+    for issue_action in IssueAction.objects.all():
+        issue_action.action_issue_type = ContentType.objects.get_for_model(Issue)
+        issue_action.action_issue_id = issue_action.issue_id
+        issue_action.save(update_fields=['action_issue_type', 'action_issue_id'])

issue/models.py

@@ -2,7 +2,7 @@
 from datetime import datetime, timedelta
 import json
 
-from django.contrib.contenttypes.fields import GenericForeignKey
+from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation
 from django.contrib.contenttypes.models import ContentType
 from django.contrib.postgres.fields import JSONField
 from django.core.serializers.json import DjangoJSONEncoder
@@ -104,6 +104,13 @@ class BaseIssue(models.Model):
 
     objects = IssueManager()
 
+    executed_actions = GenericRelation(
+        'issue.IssueAction',
+        content_type_field='action_issue_type',
+        object_id_field='action_issue_id',
+        related_query_name='+'
+    )
+
     class Meta:
         abstract = True
 
@@ -183,15 +190,19 @@ class IssueAction(models.Model):
     """
     A response that was taken to address a particular issue.
     """
-    issue = models.ForeignKey(Issue, related_name='executed_actions', on_delete=models.CASCADE)
+    # deprecated for generic issue relation
+    issue = models.ForeignKey(Issue, related_name='+', null=True, on_delete=models.CASCADE)
+    action_issue_type = models.ForeignKey(ContentType, related_name='+', null=True, on_delete=models.CASCADE)
+    action_issue_id = models.PositiveIntegerField(null=True)
+    action_issue = GenericForeignKey('action_issue_type', 'action_issue_id')
     responder_action = models.ForeignKey('issue.ResponderAction', on_delete=models.CASCADE)
     execution_time = models.DateTimeField(auto_now_add=True)
     success = models.BooleanField(default=True)
     details = JSONField(null=True, blank=True, encoder=DjangoJSONEncoder)
 
     def __str__(self):
         return (
-            'IssueResponse: {self.issue.name} - {self.responder_action} - '
+            'IssueResponse: {self.action_issue.name} - {self.responder_action} - '
             '{self.success} at {self.execution_time}'.format(self=self)
         )
 
@@ -214,6 +225,8 @@ class Responder(models.Model):
     Responder record.
     """
     watch_pattern = RegexField(null=True, max_length=128)
+    # whether to allow responder to respond, even when actions were already executed for a given issue
+    allow_retry = models.BooleanField(default=False)
 
     def __str__(self):
         return 'Responder: {watch_pattern.pattern}'.format(watch_pattern=self.watch_pattern)
@@ -244,8 +257,13 @@ def _execute(self, issue):
             ])
 
     def _get_pending_actions_for_issue(self, issue):
+        """
+        Determine which actions should be executed for the given issue
+        """
+        if self.allow_retry:
+            return self.actions.order_by('delay_sec')
+        # exclude actions that have already been executed for the given issue
         already_executed_action_pks = issue.executed_actions.values_list('responder_action__pk', flat=True).all()
-
         return self.actions.exclude(pk__in=already_executed_action_pks).order_by('delay_sec')
 
 
@@ -290,7 +308,11 @@ def execute(self, issue):
         except Exception as e:
             kwargs = self.construct_issue_action_kwargs(False, str(e))
 
-        return IssueAction(issue=issue, **kwargs)
+        return IssueAction(
+            action_issue_id=issue.id,
+            action_issue_type=ContentType.objects.get_for_model(issue),
+            **kwargs
+        )
 
     def construct_issue_action_kwargs(self, success, failure_details=None):
         """

issue/tests/model_tests.py

@@ -169,11 +169,19 @@ def test__is_wont_fix(self):
 
 class IssueActionTests(TestCase):
     def test__str__(self):
-        ia = N(IssueAction)
+        issue = G(Issue, name='bar')
+        responder = G(Responder, watch_pattern='foo')
+        responder_action = G(ResponderAction, responder=responder)
+        issue_action = N(
+            IssueAction,
+            responder_action=responder_action,
+            action_issue_type=ContentType.objects.get_for_model(Issue),
+            action_issue_id=issue.id
+        )
         self.assertEqual(
-            'IssueResponse: {self.issue.name} - {self.responder_action} - '
-            '{self.success} at {self.execution_time}'.format(self=ia),
-            str(ia)
+            'IssueResponse: {self.action_issue.name} - {self.responder_action} - '
+            '{self.success} at {self.execution_time}'.format(self=issue_action),
+            str(issue_action)
         )
 
 
@@ -233,13 +241,18 @@ def test__get_pending_actions_for_issue_ignores_executed_actions(self):
         # Setup the scenario
         now = datetime(2014, 8, 11, 15, 0, 0)
         delta = timedelta(minutes=30)
-        r = G(Responder)
-        ra = G(ResponderAction, responder=r, delay_sec=delta.total_seconds())
+        responder = G(Responder, allow_retry=False)
+        responder_action = G(ResponderAction, responder=responder, delay_sec=delta.total_seconds())
         issue = G(Issue, creation_time=now - (delta * 2))
-        G(IssueAction, issue=issue, responder_action=ra)
+        G(
+            IssueAction,
+            action_issue_id=issue.id,
+            action_issue_type=ContentType.objects.get_for_model(Issue),
+            responder_action=responder_action
+        )
 
         # Run the code and verify expectation
-        self.assertFalse(r._get_pending_actions_for_issue(issue).exists())
+        self.assertFalse(responder._get_pending_actions_for_issue(issue).exists())
 
     @patch('issue.models.load_function', spec_set=True)
     def test__execute_all_success(self, load_function):
@@ -276,9 +289,9 @@ def do_3(*args, **kwargs):
         self.assertTrue(self.do_call_time < self.do_2_call_time)
         self.assertTrue(self.do_2_call_time < self.do_3_call_time)
 
-        self.assertTrue(IssueAction.objects.filter(issue=issue, responder_action=ra).exists())
-        self.assertTrue(IssueAction.objects.filter(issue=issue, responder_action=ra2).exists())
-        self.assertTrue(IssueAction.objects.filter(issue=issue, responder_action=ra3).exists())
+        self.assertTrue(IssueAction.objects.filter(action_issue_id=issue.id, responder_action=ra).exists())
+        self.assertTrue(IssueAction.objects.filter(action_issue_id=issue.id, responder_action=ra2).exists())
+        self.assertTrue(IssueAction.objects.filter(action_issue_id=issue.id, responder_action=ra3).exists())
 
     @patch('issue.models.load_function', spec_set=True)
     def test__execute_stops_when_some_actions_are_not_yet_executable(self, load_function):
@@ -315,9 +328,9 @@ def do_3(*args, **kwargs):
         self.assertTrue(self.do_call_time < self.do_2_call_time)
         self.assertIsNone(self.do_3_call_time)
 
-        self.assertTrue(IssueAction.objects.filter(issue=issue, responder_action=ra).exists())
-        self.assertTrue(IssueAction.objects.filter(issue=issue, responder_action=ra2).exists())
-        self.assertTrue(IssueAction.objects.filter(issue=issue, responder_action=ra3).exists())
+        self.assertTrue(IssueAction.objects.filter(action_issue_id=issue.id, responder_action=ra).exists())
+        self.assertTrue(IssueAction.objects.filter(action_issue_id=issue.id, responder_action=ra2).exists())
+        self.assertTrue(IssueAction.objects.filter(action_issue_id=issue.id, responder_action=ra3).exists())
 
     @freeze_time(datetime(2014, 8, 13, 12))
     @patch('issue.models.load_function', spec_set=True)
@@ -326,9 +339,19 @@ def test__execute_resumes_after_sufficient_time(self, load_function):
         delta = timedelta(seconds=30)
         issue = G(Issue, creation_time=datetime.utcnow() - (2 * delta))
         responder = G(Responder, issue=issue)
-        ra = G(ResponderAction, responder=responder, delay_sec=0, target_function='do_1')
-        ra2 = G(ResponderAction, responder=responder, delay_sec=delta.total_seconds(), target_function='do_2')
-        G(IssueAction, issue=issue, responder_action=ra)
+        responder_action = G(ResponderAction, responder=responder, delay_sec=0, target_function='do_1')
+        responder_action2 = G(
+            ResponderAction,
+            responder=responder,
+            delay_sec=delta.total_seconds(),
+            target_function='do_2'
+        )
+        G(
+            IssueAction,
+            action_issue_id=issue.id,
+            action_issue_type=ContentType.objects.get_for_model(Issue),
+            responder_action=responder_action
+        )
 
         self.do_called = False
         self.do_2_called = False
@@ -350,8 +373,18 @@ def do_2(*args, **kwargs):
         self.assertFalse(self.do_called)
         self.assertTrue(self.do_2_called)
 
-        self.assertTrue(IssueAction.objects.filter(issue=issue, responder_action=ra).exists())
-        self.assertTrue(IssueAction.objects.filter(issue=issue, responder_action=ra2).exists())
+        self.assertTrue(
+            IssueAction.objects.filter(
+                action_issue_id=issue.id,
+                responder_action=responder_action
+            ).exists()
+        )
+        self.assertTrue(
+            IssueAction.objects.filter(
+                action_issue_id=issue.id,
+                responder_action=responder_action2
+            ).exists()
+        )
 
     @patch('issue.models.load_function', spec_set=True)
     def test__execute_failure_does_not_stop_other_actions(self, load_function):
@@ -389,21 +422,73 @@ def do_3(*args, **kwargs):
         self.assertTrue(self.do_call_time < self.do_2_call_time)
         self.assertTrue(self.do_2_call_time < self.do_3_call_time)
 
-        self.assertTrue(IssueAction.objects.filter(issue=issue, responder_action=ra).exists())
-        self.assertTrue(IssueAction.objects.filter(issue=issue, responder_action=ra2).exists())
-        self.assertTrue(IssueAction.objects.filter(issue=issue, responder_action=ra3).exists())
+        self.assertTrue(IssueAction.objects.filter(action_issue_id=issue.id, responder_action=ra).exists())
+        self.assertTrue(IssueAction.objects.filter(action_issue_id=issue.id, responder_action=ra2).exists())
+        self.assertTrue(IssueAction.objects.filter(action_issue_id=issue.id, responder_action=ra3).exists())
         self.assertEqual(
             json.dumps(str(Exception('what-an-exceptional-message'))),
-            IssueAction.objects.get(issue=issue, responder_action=ra2).details)
+            IssueAction.objects.get(action_issue_id=issue.id, responder_action=ra2).details)
+
+    @patch('issue.models.load_function', spec_set=True)
+    def test_allow_retry(self, load_function):
+        """
+        Confirms that responders that are configured to allow retry will not be restricted
+        by previous IssueActions
+        """
+        # Setup the scenario
+        issue = G(Issue)
+        responder = G(Responder, allow_retry=True)
+        # Note: we don't care what the target_function path is since we patch the load_function function
+        responder_action = G(ResponderAction, responder=responder, delay_sec=0)
+        responder_action2 = G(ResponderAction, responder=responder, delay_sec=0)
+
+        self.do_call_time = None
+        self.do_2_call_time = None
+
+        def do_1(*args, **kwargs):
+            self.do_call_time = datetime.utcnow()
+            return True
+
+        def do_2(*args, **kwargs):
+            self.do_2_call_time = datetime.utcnow()
+            return True
+
+        load_function.side_effect = [do_1, do_2]
+
+        # attempt to respond
+        responder._execute(issue)
+
+        # Verify expectations
+        self.assertEqual(2, IssueAction.objects.count())
+        self.assertTrue(
+            IssueAction.objects.filter(
+                action_issue_id=issue.id,
+                responder_action=responder_action
+            ).exists()
+        )
+        self.assertTrue(
+            IssueAction.objects.filter(
+                action_issue_id=issue.id,
+                responder_action=responder_action2
+            ).exists()
+        )
+
+        # run again
+        responder._execute(issue)
+        self.assertEqual(4, IssueAction.objects.count())
 
 
 class ResponderActionTests(TestCase):
     def test__str__(self):
-        r = G(ResponderAction)
+        responder = G(Responder, watch_pattern='foo')
+        responder_action = G(ResponderAction, responder=responder)
         self.assertEqual(
             'ResponderAction: {responder} - {target_function} - {function_kwargs}'.format(
-                responder=r.responder, target_function=r.target_function, function_kwargs=r.function_kwargs),
-            str(r)
+                responder=responder_action.responder,
+                target_function=responder_action.target_function,
+                function_kwargs=responder_action.function_kwargs
+            ),
+            str(responder_action)
         )
 
     def test_is_time_to_execute(self):
@@ -454,7 +539,7 @@ def test_execute(self, load_function):
         load_function.assert_called_with(target_function)
         load_function.return_value.assert_called_with(issue, foo='bar')
 
-        self.assertTrue(IssueAction.objects.filter(issue=issue, **expected_issue_action_kwargs).exists())
+        self.assertTrue(IssueAction.objects.filter(action_issue_id=issue.id, **expected_issue_action_kwargs).exists())
         # The 'None' that is stored as the details is first json encoded
         self.assertEqual(json.dumps(None), IssueAction.objects.get().details)
 
@@ -484,7 +569,7 @@ def test_execute_with_failure(self, load_function):
         load_function.assert_called_with(target_function)
         load_function.return_value.assert_called_with(issue, foo='bar')
 
-        self.assertTrue(IssueAction.objects.filter(issue=issue, **expected_issue_action_kwargs).exists())
+        self.assertTrue(IssueAction.objects.filter(action_issue_id=issue.id, **expected_issue_action_kwargs).exists())
         self.assertEqual(json.dumps(str(Exception('what-an-exceptional-message'))), IssueAction.objects.get().details)
 
 

issue/version.py

@@ -1 +1 @@
-__version__ = '3.2.0'
+__version__ = '3.4.1'