jira: fix add/edit product jira logic (#3366)

* jira: fix add/edit product jira logic

* jira: fix empty configs for products/engagements

* jira: show epic url for engagement

* jira: fix instance check

Author: valentijnscholten

dojo/db_migrations/0065_delete_empty_jira_project_configs.py

@@ -0,0 +1,32 @@
+# Generated by Django 2.2.16 on 2020-11-08 08:01
+
+from django.db import migrations
+import logging
+
+logger = logging.getLogger(__name__)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('dojo', '0064_jira_refactor_populate'),
+    ]
+
+    def delete_empty_jira_project_configs(apps, schema_editor):
+        logger.info('removing JIRA_Projects with empty instance and empty project key created due to bugs in 1.10.0')
+        logger.info('details in https://github.com/DefectDojo/django-DefectDojo/issues/3354')
+        # querying on null or blank or whatever can get the wrong results, so just iterate over all configs to be reliable
+        JIRA_Project = apps.get_model('dojo', 'JIRA_Project')
+        for jira_project in JIRA_Project.objects.all():
+            if not jira_project.jira_instance and not jira_project.project_key:
+                product = jira_project.product
+                product_id = product.id if product else 0
+                engagement = jira_project.engagement
+                engagement_id = engagement.id if engagement else 0
+
+                logger.info('removing empty JIRA_Project %i for product %i:%s engagement %i:%s', jira_project.id, product_id, product, engagement_id, engagement)
+                jira_project.delete()
+
+    operations = [
+        migrations.RunPython(delete_empty_jira_project_configs, migrations.RunPython.noop),
+    ]

dojo/engagement/views.py

@@ -146,17 +146,14 @@ def edit_engagement(request, eid):
     engagement = Engagement.objects.get(pk=eid)
     is_ci_cd = engagement.engagement_type == "CI/CD"
     jira_epic_form = None
-    jira_project = jira_helper.get_jira_project(engagement, use_inheritance=False)
+    jira_project = None
     jira_error = False
 
     if request.method == 'POST':
         form = EngForm(request.POST, instance=engagement, cicd=is_ci_cd, product=engagement.product.id, user=request.user)
+        jira_project = jira_helper.get_jira_project(engagement, use_inheritance=False)
 
-        jira_project_form = JIRAProjectForm(request.POST, prefix='jira-project-form', instance=jira_project, target='engagement')
-        jira_epic_form = JIRAEngagementForm(request.POST, prefix='jira-epic-form', instance=engagement)
-
-        if (form.is_valid() and (jira_project_form is None or jira_project_form.is_valid()) and (jira_epic_form is None or jira_epic_form.is_valid())):
-
+        if form.is_valid():
             # first save engagement details
             new_status = form.cleaned_data.get('status')
             engagement = form.save(commit=False)
@@ -170,72 +167,37 @@ def edit_engagement(request, eid):
             t = ", ".join('"{0}"'.format(w) for w in tags)
             engagement.tags = t
 
-            # save jira project config
-            jira_project = jira_project_form.save(commit=False)
-            jira_project.engagement = engagement
-            # only check jira project if form is sufficiently populated
-            if jira_project.jira_instance and jira_project.project_key:
-                jira_error = not jira_helper.is_jira_project_valid(jira_project)
-
-                if not jira_error:
-                    jira_project.save()
-
-                    messages.add_message(
-                        request,
-                        messages.SUCCESS,
-                        'JIRA Project config added successfully.',
-                        extra_tags='alert-success')
-
-            # push epic
-            if jira_epic_form.cleaned_data.get('push_to_jira'):
-                if jira_helper.push_to_jira(engagement):
-                    messages.add_message(
-                        request,
-                        messages.SUCCESS,
-                        'Push to JIRA for Epic queued succesfully, check alerts on the top right for errors',
-                        extra_tags='alert-success')
-                else:
-                    jira_error = True
-
-                    messages.add_message(
-                        request,
-                        messages.SUCCESS,
-                        'Push to JIRA for Epic failed, check alerts on the top right for errors',
-                        extra_tags='alert-danger')
-
             messages.add_message(
                 request,
                 messages.SUCCESS,
                 'Engagement updated successfully.',
                 extra_tags='alert-success')
 
-            if not jira_error:
+            success, jira_project_form = jira_helper.process_jira_project_form(request, instance=jira_project, engagement=engagement)
+            error = not success
+
+            success, jira_epic_form = jira_helper.process_jira_epic_form(request, engagement=engagement)
+            error = error or not success
+
+            if not error:
                 if '_Add Tests' in request.POST:
                     return HttpResponseRedirect(
                         reverse('add_tests', args=(engagement.id, )))
                 else:
                     return HttpResponseRedirect(
                         reverse('view_engagement', args=(engagement.id, )))
-
         else:
-            # if forms invalid, page will just reload and show errors
-            if jira_project_form.errors or jira_epic_form.errors:
-                messages.add_message(
-                    request,
-                    messages.ERROR,
-                    'Errors in JIRA forms, see below',
-                    extra_tags='alert-danger')
+            logger.debug(form.errors)
 
-    else:
-        form = EngForm(initial={'product': engagement.product}, instance=engagement, cicd=is_ci_cd, product=engagement.product, user=request.user)
+    form = EngForm(initial={'product': engagement.product}, instance=engagement, cicd=is_ci_cd, product=engagement.product, user=request.user)
 
-        jira_project_form = None
-        jira_epic_form = None
-        if get_system_setting('enable_jira'):
-            jira_project_form = JIRAProjectForm(prefix='jira-project-form', instance=jira_project, target='engagement', product=engagement.product)
-            if jira_project:
-                logger.debug('showing jira-epic-form')
-                jira_epic_form = JIRAEngagementForm(prefix='jira-epic-form', instance=engagement)
+    jira_project_form = None
+    jira_epic_form = None
+    if get_system_setting('enable_jira'):
+        jira_project = jira_helper.get_jira_project(engagement, use_inheritance=False)
+        jira_project_form = JIRAProjectForm(instance=jira_project, target='engagement', product=engagement.product)
+        logger.debug('showing jira-epic-form')
+        jira_epic_form = JIRAEngagementForm(instance=engagement)
 
     form.initial['tags'] = [tag.name for tag in engagement.tags]
 
@@ -248,6 +210,7 @@ def edit_engagement(request, eid):
         'edit': True,
         'jira_epic_form': jira_epic_form,
         'jira_project_form': jira_project_form,
+        'engagement': engagement,
     })
 
 

dojo/fixtures/dojo_testdata.json

@@ -1748,19 +1748,6 @@
       "project_key": "NTEST"
     }
   },
-  {
-    "pk": 3,
-    "model": "dojo.jira_project",
-    "fields": {
-      "push_notes": false,
-      "product": 3,
-      "push_all_issues": false,
-      "component": "",
-      "enable_engagement_epic_mapping": false,
-      "jira_instance": 4,
-      "project_key": "key3"
-    }
-  },
   {
     "pk": 1,
     "model": "dojo.note_type", 

dojo/forms.py

@@ -31,7 +31,6 @@
 from dojo.tools import requires_file, SCAN_SONARQUBE_API
 from dojo.user.helper import user_is_authorized
 from django.urls import reverse
-import dojo.jira_link.helper as jira_helper
 import logging
 
 logger = logging.getLogger(__name__)
@@ -2077,6 +2076,8 @@ class Meta:
 class JIRAProjectForm(forms.ModelForm):
     jira_instance = forms.ModelChoiceField(queryset=JIRA_Instance.objects.all(), label='JIRA Instance', required=False)
 
+    prefix = 'jira-project-form'
+
     class Meta:
         model = JIRA_Project
         exclude = ['product', 'engagement']
@@ -2085,18 +2086,23 @@ def __init__(self, *args, **kwargs):
         # if the form is shown for an engagement, we set a placeholder text around inherited settings from product
         self.target = kwargs.pop('target', 'product')
         self.product = kwargs.pop('product', None)
+        self.engagement = kwargs.pop('engagement', None)
         super().__init__(*args, **kwargs)
 
         # logger.debug('self.target: %s, self.product: %s, self.instance: %s', self.target, self.product, self.instance)
         if self.target == 'engagement':
-            if not self.product and self.instance and self.instance.engagement and self.instance.engagement.product:
-                self.product = self.instance.engagement.product
-            product_name = self.product.name if self.product else ''
+            product_name = self.product.name if self.product else self.engagement.product.name if self.engagement.product else ''
 
             self.fields['project_key'].widget = forms.TextInput(attrs={'placeholder': 'JIRA settings inherited from product ''%s''' % product_name})
             self.fields['project_key'].help_text = 'JIRA settings are inherited from product ''%s'', unless configured differently here.' % product_name
             self.fields['jira_instance'].help_text = 'JIRA settings are inherited from product ''%s'' , unless configured differently here.' % product_name
 
+        # if we don't have an instance, django will insert a blank empty one :-(
+        # so we have to check for id to make sure we only trigger this when there is a real instance from db
+        if self.instance.id:
+            self.fields['jira_instance'].required = True
+            self.fields['project_key'].required = True
+
     def clean(self):
         logger.debug('validating jira project form')
         cleaned_data = super().clean()
@@ -2155,6 +2161,7 @@ def __init__(self, *args, **kwargs):
         self.fields['jira_issue'].widget = forms.TextInput(attrs={'placeholder': 'Leave empty and check push to jira to create a new JIRA issue'})
 
     def clean(self):
+        import dojo.jira_link.helper as jira_helper
         logger.debug('validating jirafindingform')
         cleaned_data = super(JIRAFindingForm, self).clean()
         jira_issue_key_new = self.cleaned_data.get('jira_issue')
@@ -2221,6 +2228,8 @@ def __init__(self, *args, **kwargs):
 
 
 class JIRAEngagementForm(forms.Form):
+    prefix = 'jira-epic-form'
+
     def __init__(self, *args, **kwargs):
         self.instance = kwargs.pop('instance', None)