Fix up models, clean up code re: PR comments

awx/api/serializers.py

@@ -3492,9 +3492,7 @@ class WorkflowApprovalTemplateSerializer(UnifiedJobTemplateSerializer):
         if 'last_job' in res:
             del res['last_job']
 
-        res.update(dict(
+        res.update(jobs = self.reverse('api:workflow_approval_template_jobs_list', kwargs={'pk': obj.pk}))
-            jobs = self.reverse('api:workflow_approval_template_jobs_list', kwargs={'pk': obj.pk}),
-        ))
         return res
 
 

awx/main/migrations/0088_v360_approval_node_notifications.py

@@ -1,4 +1,4 @@
-# Generated by Django 2.2.4 on 2019-09-04 18:23
+# Generated by Django 2.2.4 on 2019-09-11 13:44
 
 from django.db import migrations, models
 
@@ -16,9 +16,9 @@ class Migration(migrations.Migration):
             field=models.ManyToManyField(blank=True, related_name='organization_notification_templates_for_approvals', to='main.NotificationTemplate'),
         ),
         migrations.AddField(
-            model_name='unifiedjobtemplate',
+            model_name='workflowjobtemplate',
             name='notification_templates_approvals',
-            field=models.ManyToManyField(blank=True, related_name='unifiedjobtemplate_notification_templates_for_approvals', to='main.NotificationTemplate'),
+            field=models.ManyToManyField(blank=True, related_name='workflowjobtemplate_notification_templates_for_approvals', to='main.NotificationTemplate'),
         ),
         migrations.AlterField(
             model_name='workflowjobnode',

awx/main/models/base.py

@@ -392,13 +392,6 @@ class NotificationFieldsModel(BaseModel):
         related_name='%(class)s_notification_templates_for_started'
     )
 
-    # Placeholder, unsure if this is required...
-    notification_templates_approvals = models.ManyToManyField(
-        "NotificationTemplate",
-        blank=True,
-        related_name='%(class)s_notification_templates_for_approvals'
-    )
-
 
 def prevent_search(relation):
     """

awx/main/models/notifications.py

@@ -463,8 +463,8 @@ class JobNotificationMixin(object):
 
     def send_notification_templates(self, status):
         from awx.main.tasks import send_notifications  # avoid circular import
-        if status not in ['pending', 'running', 'succeeded', 'failed']:
+        if status not in ['running', 'succeeded', 'failed']:
-            raise ValueError(_("status must be either pending, running, succeeded or failed"))
+            raise ValueError(_("status must be either running, succeeded or failed"))
         try:
             notification_templates = self.get_notification_templates()
         except Exception:

awx/main/models/organization.py

@@ -51,6 +51,11 @@ class Organization(CommonModel, NotificationFieldsModel, ResourceMixin, CustomVi
         default=0,
         help_text=_('Maximum number of hosts allowed to be managed by this organization.'),
     )
+    notification_templates_approvals = models.ManyToManyField(
+        "NotificationTemplate",
+        blank=True,
+        related_name='%(class)s_notification_templates_for_approvals'
+    )
 
     admin_role = ImplicitRoleField(
         parent_role='singleton:' + ROLE_SINGLETON_SYSTEM_ADMINISTRATOR,

awx/main/models/unified_jobs.py

@@ -1217,10 +1217,6 @@ class UnifiedJob(PolymorphicModel, PasswordFieldsModel, CommonModelNameNotUnique
                     status=self.status,
                     traceback=self.result_traceback)
 
-    # Placeholder...
-    # def approval_notification_data(self):
-        # for approval in WorkflowApproval.objects.filter(workflow_approval_template=instance, status='pending'):
-
     def pre_start(self, **kwargs):
         if not self.can_start:
             self.job_explanation = u'%s is not in a startable state: %s, expecting one of %s' % (self._meta.verbose_name, self.status, str(('new', 'waiting')))

awx/main/models/workflow.py

@@ -387,6 +387,12 @@ class WorkflowJobTemplate(UnifiedJobTemplate, WorkflowJobOptions, SurveyJobTempl
         blank=True,
         default=False,
     )
+    notification_templates_approvals = models.ManyToManyField(
+        "NotificationTemplate",
+        blank=True,
+        related_name='%(class)s_notification_templates_for_approvals'
+    )
+
     admin_role = ImplicitRoleField(parent_role=[
         'singleton:' + ROLE_SINGLETON_SYSTEM_ADMINISTRATOR,
         'organization.workflow_admin_role'
@@ -441,15 +447,9 @@ class WorkflowJobTemplate(UnifiedJobTemplate, WorkflowJobOptions, SurveyJobTempl
         success_notification_templates = list(base_notification_templates
                                               .filter(unifiedjobtemplate_notification_templates_for_success__in=[self]))
         approval_notification_templates = list(base_notification_templates
-                                               .filter(unifiedjobtemplate_notification_templates_for_approvals__in=[self]))
+                                               .filter(workflowjobtemplate_notification_templates_for_approvals__in=[self]))
         # Get Organization NotificationTemplates
         if self.organization is not None:
-            error_notification_templates = set(error_notification_templates + list(base_notification_templates.filter(
-                organization_notification_templates_for_errors=self.organization)))
-            started_notification_templates = set(started_notification_templates + list(base_notification_templates.filter(
-                organization_notification_templates_for_started=self.organization)))
-            success_notification_templates = set(success_notification_templates + list(base_notification_templates.filter(
-                organization_notification_templates_for_success=self.organization)))
             approval_notification_templates = set(approval_notification_templates + list(base_notification_templates.filter(
                 organization_notification_templates_for_approvals=self.organization)))
         return dict(error=list(error_notification_templates),
@@ -726,9 +726,11 @@ class WorkflowApproval(UnifiedJob, JobNotificationMixin):
         for nt in self.workflow_job_template.notification_templates["approvals"]:
             try:
                 (notification_subject, notification_body) = self.build_notification_message(nt, status)
-            except AttributeError:
+            except Exception:
-                raise NotImplementedError("build_notification_message() does not exist" % status)
+                logger.debug("build_notification_message() does not exist")
 
+            # Use kwargs to force late-binding
+            # https://stackoverflow.com/a/3431699/10669572
             def send_it(local_nt=nt, local_subject=notification_subject, local_body=notification_body):
                 def _func():
                     send_notifications.delay([local_nt.generate_notification(local_subject, local_body).id],