Skip to content

Commit c7e13a3

Browse files
Bulk edit push groups and findings fix (#12813)
* bulk edit: add test case for pushing mixed group/ungrouped findings to prove bug * cleanup * fix bulk edit mixed group/ungrouped bug * extend push conditions * rebase
1 parent d0fe234 commit c7e13a3

2 files changed

Lines changed: 106 additions & 7 deletions

File tree

dojo/finding/views.py

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2879,10 +2879,16 @@ def finding_bulk_update_all(request, pid=None):
28792879
error_counts = defaultdict(lambda: 0)
28802880
success_count = 0
28812881
finding_groups = set( # noqa: C401
2882-
find.finding_group for find in finds if find.has_finding_group
2882+
finding.finding_group
2883+
for finding in finds
2884+
if finding.has_finding_group
2885+
and (
2886+
jira_helper.is_push_all_issues(finding)
2887+
or jira_helper.is_keep_in_sync_with_jira(finding)
2888+
or form.cleaned_data.get("push_to_jira")
2889+
)
28832890
)
28842891
logger.debug("finding_groups: %s", finding_groups)
2885-
groups_pushed_to_jira = False
28862892
for group in finding_groups:
28872893
if form.cleaned_data.get("push_to_jira"):
28882894
(
@@ -2905,7 +2911,6 @@ def finding_bulk_update_all(request, pid=None):
29052911

29062912
if success_count > 0:
29072913
add_success_message_to_response(f"{success_count} finding groups pushed to JIRA successfully")
2908-
groups_pushed_to_jira = True
29092914

29102915
# refresh from db
29112916
finds = finds.all()
@@ -2928,10 +2933,11 @@ def finding_bulk_update_all(request, pid=None):
29282933
# the checkbox gets disabled and is always false
29292934
# push_to_jira = jira_helper.is_push_to_jira(new_finding,
29302935
# form.cleaned_data.get('push_to_jira'))
2931-
if not groups_pushed_to_jira and (
2932-
jira_helper.is_push_all_issues(finding)
2933-
or form.cleaned_data.get("push_to_jira")
2934-
):
2936+
if (
2937+
form.cleaned_data.get("push_to_jira")
2938+
or jira_helper.is_push_all_issues(finding)
2939+
or jira_helper.is_keep_in_sync_with_jira(finding)
2940+
) and not finding.has_finding_group:
29352941
(
29362942
can_be_pushed_to_jira,
29372943
error_message,

unittests/test_jira_import_and_pushing_api.py

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -965,6 +965,99 @@ def test_engagement_epic_mapping_disabled_no_epic_and_push_findings(self):
965965

966966
self.assert_cassette_played()
967967

968+
@patch("dojo.jira_link.helper.can_be_pushed_to_jira", return_value=(True, None, None))
969+
@patch("dojo.jira_link.helper.is_push_all_issues", return_value=False)
970+
@patch("dojo.jira_link.helper.push_to_jira", return_value=None)
971+
@patch("dojo.notifications.helper.WebhookNotificationManger.send_webhooks_notification")
972+
def test_bulk_edit_mixed_findings_and_groups_jira_push_bug(self, mock_webhooks, mock_push_to_jira, mock_is_push_all_issues, mock_can_be_pushed):
973+
"""
974+
Test the bug in bulk edit: when bulk editing findings where some are in groups
975+
and some are not, individual findings should still be pushed to JIRA even if
976+
groups are also pushed.
977+
978+
Bug: If finding groups are pushed to JIRA (groups_pushed_to_jira=True),
979+
then individual findings are skipped due to the condition:
980+
`if not groups_pushed_to_jira and (...)`
981+
"""
982+
# Import scan with groups but don't push to JIRA initially
983+
import0 = self.import_scan_with_params(
984+
self.npm_groups_sample_filename,
985+
scan_type="NPM Audit Scan",
986+
group_by="component_name+component_version",
987+
push_to_jira=False,
988+
verified=True,
989+
)
990+
test_id = import0["test"]
991+
992+
# Verify no JIRA issues were created during import
993+
self.assert_jira_issue_count_in_test(test_id, 0)
994+
self.assert_jira_group_issue_count_in_test(test_id, 0)
995+
996+
# Get the findings and finding groups created
997+
Finding.objects.filter(test__id=test_id).order_by("id")
998+
finding_groups = Finding_Group.objects.filter(test__id=test_id)
999+
1000+
# Create mixed scenario: some findings in groups, some ungrouped
1001+
# Remove one entire group to create ungrouped findings
1002+
if finding_groups.exists():
1003+
# Remove all findings from the first group and delete the group
1004+
group_to_remove = finding_groups.first()
1005+
list(group_to_remove.findings.all())
1006+
# Remove all findings from this group, making them ungrouped
1007+
group_to_remove.findings.clear()
1008+
# Delete the empty group
1009+
group_to_remove.delete()
1010+
1011+
# Verify we now have both grouped and ungrouped findings
1012+
# Note: finding_group is a cached property, so we need to check if findings are in any group
1013+
all_findings = Finding.objects.filter(test__id=test_id)
1014+
grouped_findings = [f for f in all_findings if f.finding_group is not None]
1015+
ungrouped_findings = [f for f in all_findings if f.finding_group is None]
1016+
1017+
self.assertGreater(len(grouped_findings), 0, "Should have some grouped findings")
1018+
self.assertGreater(len(ungrouped_findings), 0, "Should have some ungrouped findings")
1019+
1020+
# Use Django test client instead of RequestFactory for proper auth
1021+
from django.contrib.auth import get_user_model
1022+
1023+
# Prepare bulk edit request data
1024+
# Get the current finding IDs after group modifications
1025+
current_findings = Finding.objects.filter(test__id=test_id)
1026+
all_finding_ids = [str(f.id) for f in current_findings]
1027+
1028+
# Login as admin user who has all permissions
1029+
admin_user = get_user_model().objects.get(username="admin")
1030+
self.client.force_login(admin_user)
1031+
1032+
post_data = {
1033+
"finding_to_update": all_finding_ids,
1034+
"push_to_jira": "on", # Checkbox value when checked
1035+
# Form validation fields - all optional but need to be present
1036+
"severity": "",
1037+
"active": "",
1038+
"verified": "",
1039+
"false_p": "",
1040+
"duplicate": "",
1041+
"out_of_scope": "",
1042+
"is_mitigated": "",
1043+
"status": "", # Required for form structure
1044+
}
1045+
1046+
# Perform bulk edit using test client
1047+
self.client.post("/finding/bulk", post_data)
1048+
1049+
# Analyze what was pushed to JIRA
1050+
group_calls = [call for call in mock_push_to_jira.call_args_list if isinstance(call[0][0], Finding_Group)]
1051+
individual_calls = [call for call in mock_push_to_jira.call_args_list if isinstance(call[0][0], Finding)]
1052+
1053+
# Test expectations - both groups AND individual findings should be pushed
1054+
self.assertGreater(len(group_calls), 0, "Finding groups should be pushed to JIRA")
1055+
self.assertGreater(len(individual_calls), 0, "Individual findings should also be pushed to JIRA despite groups being pushed")
1056+
1057+
# Verify the fix: we should have exactly 2 groups + 2 individual findings pushed
1058+
self.assertEqual(len(group_calls), 2, "Expected 2 finding groups to be pushed")
1059+
self.assertEqual(len(individual_calls), 2, "Expected 2 individual findings to be pushed")
1060+
9681061
# creation of epic via the UI is already tested in test_jira_config_engagement_epic, so
9691062
# we take a shortcut here as creating an engagement with epic mapping via the API is not implemented yet
9701063
def create_engagement_epic(self, engagement):

0 commit comments

Comments
 (0)