Skip to content

Commit 6d14cbf

Browse files
fix: make retriable uploads retriable even if no error (#578)
* fix: make retriable uploads retriable even if no error * fix: update tests * fix: cursor
1 parent 5d93f94 commit 6d14cbf

File tree

2 files changed

+43
-48
lines changed

2 files changed

+43
-48
lines changed

libs/shared/shared/django_apps/upload_breadcrumbs/admin.py

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -560,9 +560,6 @@ def get_urls(self):
560560
@admin.display(description="Actions", ordering=None)
561561
def resend_upload_button(self, obj: UploadBreadcrumb) -> str:
562562
"""Display resend button in the list view for failed uploads."""
563-
if not self._is_failed_upload(obj):
564-
return "-"
565-
566563
resend_url = reverse(
567564
"admin:upload_breadcrumbs_uploadbreadcrumb_resend_upload", args=[obj.id]
568565
)
@@ -577,24 +574,16 @@ def resend_upload_action(self, obj: UploadBreadcrumb) -> str:
577574
if not obj.pk: # New object
578575
return "-"
579576

580-
html_parts = []
581-
582-
if self._is_failed_upload(obj):
583-
resend_url = reverse(
584-
"admin:upload_breadcrumbs_uploadbreadcrumb_resend_upload", args=[obj.id]
585-
)
586-
html_parts.append(
587-
f'<a class="button default" href="{resend_url}" '
588-
f"onclick=\"return confirm('Are you sure you want to resend this upload for commit {obj.commit_sha[:7]}?')\">🔄 Resend Upload</a>"
589-
"<br><br>"
590-
"<div><strong>⚠️ Note:</strong> This will create a new upload task for the same commit and repository. "
591-
"The original upload data may no longer be available in storage.</div>"
592-
)
593-
else:
594-
html_parts.append(
595-
"<div>✅ This upload does not appear to have failed. Resend option is not available.</div>"
596-
)
597-
577+
resend_url = reverse(
578+
"admin:upload_breadcrumbs_uploadbreadcrumb_resend_upload", args=[obj.id]
579+
)
580+
html_parts = [
581+
f'<a class="button default" href="{resend_url}" '
582+
f"onclick=\"return confirm('Are you sure you want to resend this upload for commit {obj.commit_sha[:7]}?')\">🔄 Resend Upload</a>"
583+
"<br><br>"
584+
"<div><strong>⚠️ Note:</strong> This will create a new upload task for the same commit and repository. "
585+
"The original upload data may no longer be available in storage.</div>"
586+
]
598587
return format_html("".join(html_parts))
599588

600589
def _is_failed_upload(self, obj: UploadBreadcrumb) -> bool:
@@ -624,10 +613,6 @@ def resend_upload_view(self, request, object_id):
624613
messages.error(request, "Upload breadcrumb not found.")
625614
return redirect("admin:upload_breadcrumbs_uploadbreadcrumb_changelist")
626615

627-
if not self._is_failed_upload(breadcrumb):
628-
messages.error(request, "This upload does not appear to have failed.")
629-
return redirect("admin:upload_breadcrumbs_uploadbreadcrumb_changelist")
630-
631616
success, error_message = self._resend_upload(breadcrumb, request.user)
632617

633618
if success:
@@ -706,6 +691,17 @@ def _resend_upload(
706691
"resend_timestamp": timezone.now().isoformat(),
707692
}
708693

694+
# Check if upload_ids is None
695+
if breadcrumb.upload_ids is None:
696+
log.error(
697+
"Cannot resend upload - breadcrumb has no upload IDs",
698+
extra={"breadcrumb_id": breadcrumb.id},
699+
)
700+
return (
701+
False,
702+
"This breadcrumb has no associated upload IDs. It may be a resend breadcrumb that hasn't been processed yet.",
703+
)
704+
709705
log.info(
710706
f"Collecting upload data for {len(breadcrumb.upload_ids)} uploads",
711707
extra={"upload_ids": breadcrumb.upload_ids},

libs/shared/shared/django_apps/upload_breadcrumbs/tests/test_admin.py

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,8 +1063,8 @@ def test_resend_upload_button_scenarios(self):
10631063
{
10641064
"milestone": Milestones.UPLOAD_COMPLETE.value,
10651065
},
1066-
None,
1067-
str,
1066+
["🔄 Resend", 'class="button"', "onclick="],
1067+
SafeString,
10681068
"successful upload",
10691069
),
10701070
]
@@ -1079,16 +1079,13 @@ def test_resend_upload_button_scenarios(self):
10791079
breadcrumb = UploadBreadcrumbFactory(breadcrumb_data=breadcrumb_data)
10801080
result = self.admin.resend_upload_button(breadcrumb)
10811081

1082-
if expected_contains is None:
1083-
self.assertEqual(result, "-", f"Failed for: {description}")
1084-
else:
1085-
self.assertIsInstance(
1086-
result, expected_type, f"Failed for: {description}"
1082+
self.assertIsInstance(
1083+
result, expected_type, f"Failed for: {description}"
1084+
)
1085+
for expected_content in expected_contains:
1086+
self.assertIn(
1087+
expected_content, result, f"Failed for: {description}"
10871088
)
1088-
for expected_content in expected_contains:
1089-
self.assertIn(
1090-
expected_content, result, f"Failed for: {description}"
1091-
)
10921089

10931090
def test_resend_upload_action_scenarios(self):
10941091
"""Test resend_upload_action with various scenarios."""
@@ -1120,10 +1117,7 @@ def test_resend_upload_action_scenarios(self):
11201117
},
11211118
"abcdef1234567890",
11221119
True,
1123-
[
1124-
"✅ This upload does not appear to have failed",
1125-
"Resend option is not available",
1126-
],
1120+
["🔄 Resend Upload", 'class="button default"', "abcdef1", "⚠️ Note:"],
11271121
"successful upload",
11281122
),
11291123
({}, "abcdef1234567890", False, None, "new object without pk"),
@@ -1170,22 +1164,27 @@ def test_resend_upload_view_with_nonexistent_breadcrumb(self, mock_messages_erro
11701164
request, "Upload breadcrumb not found."
11711165
)
11721166

1173-
@patch("django.contrib.messages.error")
1174-
def test_resend_upload_view_with_non_failed_upload(self, mock_messages_error):
1175-
"""Test resend_upload_view handles non-failed uploads."""
1167+
@patch("django.contrib.messages.success")
1168+
def test_resend_upload_view_with_non_failed_upload(self, mock_messages_success):
1169+
"""Test resend_upload_view allows resending non-failed uploads."""
11761170
breadcrumb_data = {
11771171
"milestone": Milestones.UPLOAD_COMPLETE.value,
11781172
}
1179-
breadcrumb = UploadBreadcrumbFactory(breadcrumb_data=breadcrumb_data)
1173+
breadcrumb = UploadBreadcrumbFactory(
1174+
breadcrumb_data=breadcrumb_data, commit_sha="abcdef1234567890"
1175+
)
11801176
request = MagicMock()
11811177
request.user = self.user
11821178

1183-
with patch.object(self.admin, "get_object", return_value=breadcrumb):
1179+
with (
1180+
patch.object(self.admin, "get_object", return_value=breadcrumb),
1181+
patch.object(self.admin, "_resend_upload", return_value=(True, None)),
1182+
):
11841183
response = self.admin.resend_upload_view(request, str(breadcrumb.id))
11851184

1186-
mock_messages_error.assert_called_once_with(
1187-
request, "This upload does not appear to have failed."
1188-
)
1185+
mock_messages_success.assert_called_once()
1186+
success_message = mock_messages_success.call_args[0][1]
1187+
self.assertIn("Upload resend triggered successfully", success_message)
11891188

11901189
@patch("django.contrib.messages.success")
11911190
def test_resend_upload_view_successful_resend(self, mock_messages_success):

0 commit comments

Comments
 (0)