fix(admin-feedback): mask email on detail + no-email indicator + await reply email send #116

Merged

YousifShkara merged 1 commit from fix/feedback-detail-mask-email-and-await-send into main 2026-06-11 08:15:52 +02:00

Conversation 0 Commits 1 Files changed 3 +54 -8

YousifShkara commented 2026-06-11 08:15:49 +02:00

Owner

Copy link

Closes BUNYIP-94. Stacks on BUNYIP-93.

Two issues on the admin feedback detail subpage after BUNYIP-85 / 92 / 93 shipped:

1. The "From" line on the detail subpage rendered the submitter's unmasked email. Admins do not need the raw address to reply (the API holds the email and routes the response server-side), and surfacing it on the detail page violates the same masking posture the row list uses.

2. Replying did not deliver an email to the submitter in at least one case. The API's send_feedback_response is wired and the template exists, but respond_to_feedback tokio::spawn'd the email send and returned 200 immediately, so any SMTP / config / template failure landed only in the spawned task's tracing logs - after the request had already returned - and was invisible to anyone tailing API logs by request_id.

Changes:

• bunyip-web/src/api/types.rs::AdminFeedbackDetail: add email_masked: Option<String> with serde(default). The API already emits it; bunyip-web just did not bind it.

• bunyip-web/src/handlers/admin.rs::feedback_detail_view: render email_masked instead of email in the identity line. Compute a has_email boolean from the raw email so the Response card can warn when the row has no recipient: a small amber banner says "No email on record - the submitter did not provide an address. Your response will be saved but cannot be delivered." Removes the silent failure mode where an admin types a thoughtful reply for a submitter who never gave an email.

• bunyip-api/src/handlers/feedback.rs::respond_to_feedback: drop the tokio::spawn around email_service.send_feedback_response(...). Await it directly. On failure, log at tracing::error! with the request's request_id, feedback_id, AND recipient (so an admin investigating "why didn't it arrive?" has the exact address that was tried). Continue returning success either way because the response IS saved to DB regardless; the change is purely visibility.

The trade-off: the respond request now blocks on SMTP. lettre's transport has its own timeouts so the upper bound is bounded; admins reply at human cadence so the extra latency is invisible. Spawning was useful when email was synchronous and slow; awaiting matches every other email-emit site in this codebase (welcome, invite, password reset).

Out of scope: a richer respond response that surfaces email_sent to the SSR so the toast reads "Response sent" vs "Response saved (email failed)". Worth doing as a follow-up if logs alone are not enough. The reorder-email-first-then-save approach is also out of scope (cleaner but requires changing the email service signature so the response text doesn't have to live on feedback.admin_response at send time).

just check-container clean (modulo the pre-existing test_config_defaults flake on main; 188 other tests pass; fmt + clippy + build clean).

#BUNYIP-94

Closes BUNYIP-94. Stacks on BUNYIP-93. Two issues on the admin feedback detail subpage after BUNYIP-85 / 92 / 93 shipped: 1. The "From" line on the detail subpage rendered the submitter's unmasked email. Admins do not need the raw address to reply (the API holds the email and routes the response server-side), and surfacing it on the detail page violates the same masking posture the row list uses. 2. Replying did not deliver an email to the submitter in at least one case. The API's send_feedback_response is wired and the template exists, but `respond_to_feedback` `tokio::spawn`'d the email send and returned 200 immediately, so any SMTP / config / template failure landed only in the spawned task's tracing logs - after the request had already returned - and was invisible to anyone tailing API logs by request_id. Changes: - bunyip-web/src/api/types.rs::AdminFeedbackDetail: add `email_masked: Option<String>` with serde(default). The API already emits it; bunyip-web just did not bind it. - bunyip-web/src/handlers/admin.rs::feedback_detail_view: render `email_masked` instead of `email` in the identity line. Compute a `has_email` boolean from the raw email so the Response card can warn when the row has no recipient: a small amber banner says "No email on record - the submitter did not provide an address. Your response will be saved but cannot be delivered." Removes the silent failure mode where an admin types a thoughtful reply for a submitter who never gave an email. - bunyip-api/src/handlers/feedback.rs::respond_to_feedback: drop the tokio::spawn around `email_service.send_feedback_response(...)`. Await it directly. On failure, log at `tracing::error!` with the request's request_id, feedback_id, AND recipient (so an admin investigating "why didn't it arrive?" has the exact address that was tried). Continue returning success either way because the response IS saved to DB regardless; the change is purely visibility. The trade-off: the respond request now blocks on SMTP. lettre's transport has its own timeouts so the upper bound is bounded; admins reply at human cadence so the extra latency is invisible. Spawning was useful when email was synchronous and slow; awaiting matches every other email-emit site in this codebase (welcome, invite, password reset). Out of scope: a richer respond response that surfaces `email_sent` to the SSR so the toast reads "Response sent" vs "Response saved (email failed)". Worth doing as a follow-up if logs alone are not enough. The reorder-email-first-then-save approach is also out of scope (cleaner but requires changing the email service signature so the response text doesn't have to live on `feedback.admin_response` at send time). `just check-container` clean (modulo the pre-existing test_config_defaults flake on main; 188 other tests pass; fmt + clippy + build clean). #BUNYIP-94

YousifShkara self-assigned this 2026-06-11 08:15:49 +02:00

YousifShkara added 1 commit 2026-06-11 08:15:49 +02:00

fix(admin-feedback): mask email on detail + no-email indicator + await reply email send ... 425a531d04

Closes BUNYIP-94. Stacks on BUNYIP-93.

Two issues on the admin feedback detail subpage after BUNYIP-85 / 92 / 93 shipped:

1. The "From" line on the detail subpage rendered the submitter's unmasked email. Admins do not need the raw address to reply (the API holds the email and routes the response server-side), and surfacing it on the detail page violates the same masking posture the row list uses.

2. Replying did not deliver an email to the submitter in at least one case. The API's send_feedback_response is wired and the template exists, but `respond_to_feedback` `tokio::spawn`'d the email send and returned 200 immediately, so any SMTP / config / template failure landed only in the spawned task's tracing logs - after the request had already returned - and was invisible to anyone tailing API logs by request_id.

Changes:

- bunyip-web/src/api/types.rs::AdminFeedbackDetail: add `email_masked: Option<String>` with serde(default). The API already emits it; bunyip-web just did not bind it.

- bunyip-web/src/handlers/admin.rs::feedback_detail_view: render `email_masked` instead of `email` in the identity line. Compute a `has_email` boolean from the raw email so the Response card can warn when the row has no recipient: a small amber banner says "No email on record - the submitter did not provide an address. Your response will be saved but cannot be delivered." Removes the silent failure mode where an admin types a thoughtful reply for a submitter who never gave an email.

- bunyip-api/src/handlers/feedback.rs::respond_to_feedback: drop the tokio::spawn around `email_service.send_feedback_response(...)`. Await it directly. On failure, log at `tracing::error!` with the request's request_id, feedback_id, AND recipient (so an admin investigating "why didn't it arrive?" has the exact address that was tried). Continue returning success either way because the response IS saved to DB regardless; the change is purely visibility.

The trade-off: the respond request now blocks on SMTP. lettre's transport has its own timeouts so the upper bound is bounded; admins reply at human cadence so the extra latency is invisible. Spawning was useful when email was synchronous and slow; awaiting matches every other email-emit site in this codebase (welcome, invite, password reset).

Out of scope: a richer respond response that surfaces `email_sent` to the SSR so the toast reads "Response sent" vs "Response saved (email failed)". Worth doing as a follow-up if logs alone are not enough. The reorder-email-first-then-save approach is also out of scope (cleaner but requires changing the email service signature so the response text doesn't have to live on `feedback.admin_response` at send time).

`just check-container` clean (modulo the pre-existing test_config_defaults flake on main; 188 other tests pass; fmt + clippy + build clean).

#BUNYIP-94

YousifShkara merged commit a3aa9d1357 into main 2026-06-11 08:15:52 +02:00

YousifShkara deleted branch fix/feedback-detail-mask-email-and-await-send 2026-06-11 08:15:52 +02:00

YousifShkara referenced this pull request from a commit 2026-06-11 08:15:53 +02:00

Merge pull request 'fix(admin-feedback): mask email on detail + no-email indicator + await reply email send' (#116) from fix/feedback-detail-mask-email-and-await-send into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

YousifShkara

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

psa-systems/bunyip!116

No description provided.