Found while reviewing #3638 (surfaced by the Greptile review on that PR). Out of scope there — that PR is a Message-Id threading fix — so filing separately.
helpdesk/api/ticket.py::bulk_reply wraps each reply_via_agent call in a blanket except Exception that writes an Error Log and moves on, and the function returns nothing. An agent who bulk-replies to 20 tickets and has 19 fail (SMTP down, no outgoing account, bad recipient) sees the same silent success as one where all 20 went out. The only trace is in Error Log, which agents do not read.
for doc in tickets:
try:
doc.reply_via_agent(message, to=doc.raised_by, attachments=attachments or [])
except Exception as e:
frappe.log_error(
title=f"Bulk reply failed for ticket {doc.name}",
message=str(e),
)
Second-order effect: link_attachments_to_tickets runs once for the whole batch before any reply is attempted, so a ticket whose reply failed keeps the File rows attached to it with no reply carrying them.
Suggested shape (not prescriptive):
- return the per-ticket outcome from
bulk_reply so the frontend can report "17 of 20 sent"
- surface the failure reason in that payload rather than only in Error Log
Not a regression from #3638 — that PR only moved the link_attachments_to_tickets call after the permission checks, which is strictly an improvement.
Found while reviewing #3638 (surfaced by the Greptile review on that PR). Out of scope there — that PR is a Message-Id threading fix — so filing separately.
helpdesk/api/ticket.py::bulk_replywraps eachreply_via_agentcall in a blanketexcept Exceptionthat writes an Error Log and moves on, and the function returns nothing. An agent who bulk-replies to 20 tickets and has 19 fail (SMTP down, no outgoing account, bad recipient) sees the same silent success as one where all 20 went out. The only trace is in Error Log, which agents do not read.Second-order effect:
link_attachments_to_ticketsruns once for the whole batch before any reply is attempted, so a ticket whose reply failed keeps the File rows attached to it with no reply carrying them.Suggested shape (not prescriptive):
bulk_replyso the frontend can report "17 of 20 sent"Not a regression from #3638 — that PR only moved the
link_attachments_to_ticketscall after the permission checks, which is strictly an improvement.