move save call out of jira helper

Author: valentijnscholten

dojo/finding/views.py

@@ -1437,7 +1437,9 @@ def reopen_finding(request, fid):
         status.save()
     # Clear the risk acceptance, if present
     ra_helper.risk_unaccept(request.user, finding)
-    jira_helper.save_and_push_to_jira(finding)
+    finding.save(dedupe_option=False, push_to_jira=False)
+    if jira_helper.is_push_all_issues(finding) or jira_helper.is_keep_in_sync_with_jira(finding):
+        jira_helper.push_to_jira(finding)
 
     reopen_external_issue(finding, "re-opened by defectdojo", "github")
 

dojo/jira_link/helper.py

@@ -109,6 +109,19 @@ def is_push_all_issues(instance):
     return None
 
 
+def is_keep_in_sync_with_jira(finding):
+    keep_in_sync_enabled = False
+    # Check if there is a jira issue that needs to be updated
+    jira_issue_exists = finding.has_jira_issue or (finding.finding_group and finding.finding_group.has_jira_issue)
+    # Only push if the finding is not in a group
+    if jira_issue_exists:
+        # Determine if any automatic sync should occur
+        keep_in_sync_enabled = get_jira_instance(finding).finding_jira_sync
+
+    return keep_in_sync_enabled
+
+
+
 def _safely_get_finding_group_status(finding_group: Finding_Group) -> str:
     # Accommodating a strange behavior where a finding group sometimes prefers `obj.status` rather than `obj.status()`
     try:
@@ -674,6 +687,8 @@ def push_to_jira(obj, *args, **kwargs):
         raise ValueError(msg)
 
     if isinstance(obj, Finding):
+        if obj.finding_group:
+            return push_finding_group_to_jira(obj.finding_group)
         return push_finding_to_jira(obj, *args, **kwargs)
 
     if isinstance(obj, Finding_Group):
@@ -1744,23 +1759,3 @@ def process_resolution_from_jira(finding, resolution_id, resolution_name, assign
     if status_changed:
         finding.save()
     return status_changed
-
-
-def save_and_push_to_jira(finding):
-    # Manage the jira status changes
-    push_to_jira_decision = False
-    # Determine if the finding is in a group. if so, not push to jira yet
-    finding_in_group = finding.has_finding_group
-    # Check if there is a jira issue that needs to be updated
-    jira_issue_exists = finding.has_jira_issue or (finding.finding_group and finding.finding_group.has_jira_issue)
-    # Only push if the finding is not in a group
-    if jira_issue_exists:
-        # Determine if any automatic sync should occur
-        push_to_jira_decision = is_push_all_issues(finding) \
-            or get_jira_instance(finding).finding_jira_sync
-    # Save the finding
-    finding.save(dedupe_option=False, product_grading_option=False, issue_updater_option=False, push_to_jira=(push_to_jira_decision and not finding_in_group))
-    # we only push the group after saving the finding to make sure
-    # the updated data of the finding is pushed as part of the group
-    if push_to_jira_decision and finding_in_group:
-        push_to_jira(finding.finding_group)

dojo/risk_acceptance/helper.py

@@ -33,7 +33,9 @@ def expire_now(risk_acceptance):
                 if risk_acceptance.restart_sla_expired:
                     finding.sla_start_date = timezone.now().date()
                 # this method both saves and pushed to JIRA (no other post processing)
-                jira_helper.save_and_push_to_jira(finding)
+                finding.save(dedupe_option=False)
+                if jira_helper.is_push_all_issues(finding) or jira_helper.is_keep_in_sync_with_jira(finding):
+                    jira_helper.push_to_jira(finding)
 
                 reactivated_findings.append(finding)
             else:
@@ -72,7 +74,9 @@ def reinstate(risk_acceptance, old_expiration_date):
                 # Update any endpoint statuses on each of the findings
                 update_endpoint_statuses(finding, accept_risk=True)
                 # this method both saves and pushed to JIRA (no other post processing)
-                jira_helper.save_and_push_to_jira(finding)
+                finding.save(dedupe_option=False)
+                if jira_helper.is_push_all_issues(finding) or jira_helper.is_keep_in_sync_with_jira(finding):
+                    jira_helper.push_to_jira(finding)
                 reinstated_findings.append(finding)
             else:
                 logger.debug("%i:%s: already inactive, not making any changes", finding.id, finding)
@@ -113,7 +117,10 @@ def remove_finding_from_risk_acceptance(user: Dojo_User, risk_acceptance: Risk_A
     # Update any endpoint statuses on each of the findings
     update_endpoint_statuses(finding, accept_risk=False)
     # this method both saves and pushed to JIRA (no other post processing)
-    jira_helper.save_and_push_to_jira(finding)
+    finding.save(dedupe_option=False)
+    if jira_helper.is_push_all_issues(finding) or jira_helper.is_keep_in_sync_with_jira(finding):
+        jira_helper.push_to_jira(finding)
+
     # best effort jira integration, no status changes
     post_jira_comments(risk_acceptance, [finding], unaccepted_message_creator)
     # Add a note to reflect that the finding was removed from the risk acceptance
@@ -319,6 +326,9 @@ def simple_risk_accept(user: Dojo_User, finding: Finding, *, perform_save=True)
         finding.save(dedupe_option=False)
     # post_jira_comment might reload from database so see unaccepted finding. but the comment
     # only contains some text so that's ok
+    if jira_helper.is_push_all_issues(finding) or jira_helper.is_keep_in_sync_with_jira(finding):
+        jira_helper.push_to_jira(finding)
+
     post_jira_comment(finding, accepted_message_creator)
     # Add a note to reflect that the finding was removed from the risk acceptance
     if user is not None:
@@ -349,7 +359,8 @@ def risk_unaccept(user: Dojo_User, finding: Finding, *, perform_save=True, post_
             post_jira_comment(finding, unaccepted_message_creator)
 
         # Update the JIRA obect for this finding
-        jira_helper.save_and_push_to_jira(finding)
+        if jira_helper.is_push_all_issues(finding) or jira_helper.is_keep_in_sync_with_jira(finding):
+            jira_helper.push_to_jira(finding)
 
         # Add a note to reflect that the finding was removed from the risk acceptance
         if user is not None: