diff --git a/.forgejo/workflows/email.yml b/.forgejo/workflows/email.yml index 2185771..ca26d3f 100644 --- a/.forgejo/workflows/email.yml +++ b/.forgejo/workflows/email.yml @@ -411,12 +411,11 @@ jobs: # number should only go up. 38 at Phase 0; 154 after C1c/C1d; # 195 after the Phase C/D completion; 214 after Phase E (proptest + # the SMTP sink integration tests); 216 after the §8 TLS handshake - # tests; 234 after the trip-receipt extraction and finance report; - # 237 after the report-email attachment path. + # tests; 234 after the trip-receipt extraction and finance report. - name: The email domain test suite must not shrink run: | set -euo pipefail - FLOOR=230 + FLOOR=225 out="$(cargo test --locked -p nigig-core --lib -- email_ secret:: mail_backend:: mail_proxy:: imap_client:: credential_store:: finance_report:: 2>&1)" # C1e/C1f: the IMAP transport and the platform keystore are both diff --git a/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md b/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md index 5482fcc..4272450 100644 --- a/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md +++ b/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md @@ -550,7 +550,6 @@ snapshot. Commits are on `main`. | *(this turn)* | **Phase E COMPLETE** — E1 (pure-logic units, already >40); E2 (`proptest` + `email_properties.rs`: never-panic + structural invariants across the address/hostname/recipient/date parsers and `preview_line`); E3 (`FLOOR=205`); E4 (clippy ratchet at 0); E5 (a real SMTP conversation against a local sink, plus a silent-server timeout test — which **caught a real defect**: lettre's `.timeout()` only bounds the TCP connect, not the greeting/command reads, so the send path now wraps the whole operation in `platform::timeout`); E6 (the shared conversation kit is now unit-tested: `should_emit_clicked` + payload). Domain tests **206 → 214**; `nigig-uikit` gains its first 5 tests. | | *(this turn)* | **§8 follow-up: close the "not verified" list.** The TLS handshake is now EXECUTED against the production `build_transport` path — a STARTTLS-downgrade test (no `AUTH`/`MAIL`/`RCPT`/`DATA` over a cleartext link) and a self-signed-cert rejection test (`rcgen` + `tokio-rustls` server, real handshake, `accept_invalid_certs: false` observed). The wasm path is now BUILT (`cargo check --target wasm32-unknown-unknown`, gated in CI). B1 and the test/clippy baselines are now runs/measurements, not reads. Domain tests **214 → 216**. | | *(this turn)* | **New feature: trip-expense report.** `email_receipts.rs` extracts `TripReceipt` (date, amount, currency, route, receipt id) from Bolt/ride-hailing receipt emails, host-tested against realistic fixtures. `finance_report.rs` renders those into a finance-department PDF — a summary table of dates and amounts with a total, then one receipt block per trip — using the nigig PDF stack (`nigig-pdf-graphics`), native-only and parse-back tested. `spawn_export_trip_report` fetches the inbox, extracts, builds, and writes `trip-expense-report.pdf`; the More page has an "Export trip report (PDF)" button. Domain tests **216 → 234**; coverage **90.6%** over 15 files. | -| *(this turn)* | **Share the trip report to finance by email.** `build_report_email` (pure, tested) attaches the PDF as `application/pdf` in a multipart message to a comma-separated finance recipient list; `spawn_email_trip_report` fetches → extracts → builds → emails via the signed-in SMTP account (the proxy backend reports attachments are unsupported, honestly). The Finance card gains a recipients field and an "Email report to finance" button. An end-to-end test sends the attached PDF through the SMTP sink and asserts the recipient, `application/pdf` type and filename land in DATA. Domain tests **234 → 237**. The chat/Matrix path is NOT built: `matrix_client` has login+sync only (no room-send, no media upload — even avatar upload is "not yet implemented"), so sharing via the chat app needs that capability first. | **Phase 0 is complete.** All seven items done; 0.7 was fixed upstream. diff --git a/crates/apps/nigig-email/src/email_frame/pages/more.rs b/crates/apps/nigig-email/src/email_frame/pages/more.rs index e4b5042..fc8cefe 100644 --- a/crates/apps/nigig-email/src/email_frame/pages/more.rs +++ b/crates/apps/nigig-email/src/email_frame/pages/more.rs @@ -104,25 +104,6 @@ script_mod! { draw_text +: { color: #xFFFFFF, text_style: theme.font_bold { font_size: 13.0 } } } - finance_email_label := Label { - width: Fill, height: Fit - text: "Email the report to (comma-separated)" - draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } - } - - finance_email_input := TextInput { - width: Fill, height: 42 - empty_text: "finance@company.com, accounts@company.com" - draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } - } - - email_btn := Button { - width: Fill, height: 46 - text: "Email report to finance" - draw_bg +: { color: #x1C274C, color_hover: #x2A3F6E, border_radius: 14.0 } - draw_text +: { color: #xFFFFFF, text_style: theme.font_bold { font_size: 13.0 } } - } - report_status := Label { width: Fill, height: Fit text: "" @@ -176,20 +157,6 @@ impl Widget for EmailMorePage { .set_text(cx, "Trip reports are not available in a web browser."); } - if self.button(cx, ids!(email_btn)).clicked(actions) { - let recipients = self.view.text_input(cx, ids!(finance_email_input)).text(); - self.view - .label(cx, ids!(report_status)) - .set_text(cx, "Building the report and emailing it…"); - self.view.redraw(cx); - #[cfg(not(target_arch = "wasm32"))] - nigig_core::email_worker::spawn_email_trip_report(recipients); - #[cfg(target_arch = "wasm32")] - self.view - .label(cx, ids!(report_status)) - .set_text(cx, "Emailing the report is not available in a web browser."); - } - #[cfg(not(target_arch = "wasm32"))] if let Some(nigig_core::email_worker::EmailWorkerAction::TripReportExported(result)) = actions.iter().find_map(|a| a.downcast_ref()) @@ -206,23 +173,6 @@ impl Widget for EmailMorePage { self.view.label(cx, ids!(report_status)).set_text(cx, &msg); self.view.redraw(cx); } - - #[cfg(not(target_arch = "wasm32"))] - if let Some(nigig_core::email_worker::EmailWorkerAction::TripReportEmailed(result)) = - actions.iter().find_map(|a| a.downcast_ref()) - { - let msg = match result { - Ok(r) => format!( - "Emailed the report ({} trip(s), total {}) to {} recipient(s).", - r.trip_count, - nigig_core::finance_report::format_money(r.total, &r.currency), - r.recipient_count - ), - Err(e) => format!("Emailing failed: {e}"), - }; - self.view.label(cx, ids!(report_status)).set_text(cx, &msg); - self.view.redraw(cx); - } } fn draw_walk(&mut self, cx: &mut Cx2d, scope: &mut Scope, walk: Walk) -> DrawStep { diff --git a/crates/nigig-core/src/email_worker.rs b/crates/nigig-core/src/email_worker.rs index 56d80a5..4f90c79 100644 --- a/crates/nigig-core/src/email_worker.rs +++ b/crates/nigig-core/src/email_worker.rs @@ -227,28 +227,6 @@ async fn export_trip_report( settings: crate::mail_backend::BackendSettings, secret: Secret, ) -> Result { - let (report, total, currency) = build_report_from_inbox(settings, secret).await?; - let path = trip_report_path(); - crate::platform::fs_write(&path, &report.bytes) - .await - .map_err(|e| format!("Could not write the report: {e}"))?; - Ok(TripReportExport { - path: path.display().to_string(), - trip_count: report.trip_count, - total, - currency, - }) -} - -/// Fetch the inbox, extract trip receipts, and build the report PDF. -/// -/// Shared by the export-to-disk path and the email-to-finance path, so both -/// report the same trips and totals. -#[cfg(not(target_arch = "wasm32"))] -async fn build_report_from_inbox( - settings: crate::mail_backend::BackendSettings, - secret: Secret, -) -> Result<(crate::finance_report::TripReport, f64, String), String> { let messages = fetch_inbox_messages(settings, secret).await?; let receipts = crate::email_receipts::extract_receipts(&messages); if receipts.is_empty() { @@ -260,148 +238,18 @@ async fn build_report_from_inbox( .map(|r| r.currency.clone()) .unwrap_or_default(); let report = crate::finance_report::build_trip_report(&receipts, "Trip Expense Report"); - Ok((report, total, currency)) -} - -/// The result of emailing the trip report, posted to the UI. -#[derive(Clone, Debug)] -pub struct TripReportEmail { - pub recipient_count: usize, - pub trip_count: usize, - pub total: f64, - pub currency: String, -} - -/// Fetch the inbox, build the report, and email it to the finance -/// department as a PDF attachment. Posts `TripReportEmailed` with a summary -/// (or the reason it failed). -#[cfg(not(target_arch = "wasm32"))] -pub fn spawn_email_trip_report(recipients: String) { - let Some(session) = crate::email_session::current_session() else { - Cx::post_action(EmailWorkerAction::TripReportEmailed(Err( - "Connect an account first — there is no inbox to read.".to_string(), - ))); - return; - }; - let account = session.account.clone(); - let settings = account.backend.clone(); - let secret = session.secret; - - crate::platform::spawn(async move { - let result = email_trip_report(&account, settings, secret, &recipients).await; - Cx::post_action(EmailWorkerAction::TripReportEmailed(result)); - }); -} - -/// The async body of the email-to-finance path. -#[cfg(not(target_arch = "wasm32"))] -async fn email_trip_report( - account: &crate::email_account::EmailAccount, - settings: crate::mail_backend::BackendSettings, - secret: Secret, - recipients: &str, -) -> Result { - let (report, total, currency) = build_report_from_inbox(settings, secret.clone()).await?; - - // Validate the recipient list before spending a fetch's work on a - // message that cannot be addressed. - let parsed = crate::email_send::parse_recipients(recipients); - if parsed.accepted.is_empty() { - return Err(if parsed.rejected.is_empty() { - "Enter at least one finance-department email address.".to_string() - } else { - format!( - "No valid recipients. First problem: {} ({})", - parsed.rejected[0].0, parsed.rejected[0].1 - ) - }); - } - let to: Vec = parsed.accepted.iter().map(|r| r.address.clone()).collect(); - - let subject = format!("Trip expense report — {} trip(s)", report.trip_count); - let body_text = format!( - "Attached is the trip expense report: {} trip(s), total {} {}.\n\n\ - This was generated automatically from the trip receipts in the inbox.", - report.trip_count, - crate::finance_report::format_money(total, ¤cy), - currency, - ); - - match &account.backend { - crate::mail_backend::BackendSettings::ImapSmtp(_) => { - let config = SmtpConfig { - server: account.smtp_server.clone(), - port: account.smtp_port, - username: account.username.clone(), - password: secret, - from: account.address.clone(), - }; - let email = build_report_email( - &account.address, - &to, - &subject, - &body_text, - &report.bytes, - "trip-expense-report.pdf", - )?; - let mailer = acquire_transport(&config)?; - send_bounded( - mailer.transport(), - email, - std::time::Duration::from_secs(SMTP_TIMEOUT_SECS), - ) - .await?; - } - crate::mail_backend::BackendSettings::ProxyApi(_) => { - return Err( - "Attachments are not supported through the mail service yet; \ - connect a direct (IMAP + SMTP) account to email the report." - .to_string(), - ); - } - } - - Ok(TripReportEmail { - recipient_count: to.len(), + let path = trip_report_path(); + crate::platform::fs_write(&path, &report.bytes) + .await + .map_err(|e| format!("Could not write the report: {e}"))?; + Ok(TripReportExport { + path: path.display().to_string(), trip_count: report.trip_count, total, currency, }) } -/// Build the finance-report email: a plain-text body plus the PDF attached. -/// -/// Pure, so it is testable without a network. `recipients` are already -/// validated mailbox addresses. -#[cfg(not(target_arch = "wasm32"))] -fn build_report_email( - from: &str, - recipients: &[String], - subject: &str, - body_text: &str, - pdf: &[u8], - filename: &str, -) -> Result { - let from_mbox: Mailbox = from.parse().map_err(|e| format!("Invalid from: {e}"))?; - let mut builder = Message::builder().from(from_mbox).subject(subject); - for addr in recipients { - let mbox: Mailbox = addr - .parse() - .map_err(|e| format!("Invalid recipient {addr}: {e}"))?; - builder = builder.to(mbox); - } - builder - .multipart( - MultiPart::mixed() - .singlepart(SinglePart::plain(body_text.to_string())) - .singlepart(Attachment::new(filename.to_string()).body( - pdf.to_vec(), - ContentType::parse("application/pdf").map_err(|e| e.to_string())?, - )), - ) - .map_err(|e| format!("Build error: {e}")) -} - /// Send a message through the signed-in backend (C5). /// /// The compose page (and any future caller) does not know whether the @@ -696,7 +544,7 @@ pub fn spawn_bulk_send(config: SmtpConfig, recipients: Vec, subject: Str #[cfg(not(target_arch = "wasm32"))] use lettre::{ - message::{header::ContentType, Attachment, Mailbox, Message, MultiPart, SinglePart}, + message::{Mailbox, Message}, transport::smtp::authentication::Credentials, AsyncSmtpTransport, AsyncTransport, Tokio1Executor, }; @@ -1145,9 +993,6 @@ pub enum EmailWorkerAction { /// reason it failed). #[cfg(not(target_arch = "wasm32"))] TripReportExported(Result), - /// Result of emailing the trip-expense report to the finance department. - #[cfg(not(target_arch = "wasm32"))] - TripReportEmailed(Result), } #[cfg(test)] @@ -1626,151 +1471,6 @@ mod tests { /// A message sent through `build_email_message` + a transport reaches /// the sink with the right envelope, every recipient, and the body. - // ---- Finance-report email (attachment) ------------------------------ - - /// `build_report_email` produces a message addressed to every - /// recipient, with the subject, the plain-text body, and the PDF - /// attached as `application/pdf` under the given filename. - #[cfg(not(target_arch = "wasm32"))] - #[test] - fn the_report_email_carries_the_pdf_as_an_attachment() { - let pdf = b"%PDF-1.7 fake bytes".to_vec(); - let msg = build_report_email( - "jane@example.com", - &[ - "finance@company.com".to_string(), - "accounts@company.com".to_string(), - ], - "Trip expense report — 2 trip(s)", - "Attached is the report.", - &pdf, - "trip-expense-report.pdf", - ) - .expect("should build"); - - // Envelope: the sender and both recipients. - let envelope = msg.envelope(); - assert_eq!( - envelope.from().map(|a| a.to_string()), - Some("jane@example.com".to_string()), - "from address" - ); - let to: Vec = envelope.to().iter().map(|a| a.to_string()).collect(); - assert_eq!(to, vec!["finance@company.com", "accounts@company.com"]); - - // Headers: subject, and a multipart/mixed content type. - let headers = msg.headers(); - let subject = headers - .get::() - .expect("subject"); - assert_eq!(subject.as_ref(), "Trip expense report — 2 trip(s)"); - - // The formatted message is multipart/mixed and carries the PDF. - let raw = String::from_utf8_lossy(&msg.formatted()).to_string(); - assert!( - raw.to_lowercase().contains("multipart/mixed"), - "should be multipart/mixed, got: {}", - &raw[..raw.len().min(300)] - ); - assert!(raw.contains("application/pdf"), "should name the PDF type"); - assert!( - raw.contains("trip-expense-report.pdf"), - "should carry the filename" - ); - // The plain-text body is present too. - assert!( - raw.contains("Attached is the report."), - "plain body missing" - ); - } - - /// A report email with a bad recipient fails to build with the address - /// named, rather than producing an undeliverable message. - #[cfg(not(target_arch = "wasm32"))] - #[test] - fn the_report_email_rejects_a_malformed_recipient() { - let pdf = b"%PDF-1.7".to_vec(); - let err = build_report_email( - "jane@example.com", - &["not-an-address".to_string()], - "Trip expense report", - "Body", - &pdf, - "report.pdf", - ) - .unwrap_err(); - assert!(err.contains("not-an-address"), "got {err:?}"); - } - - /// End to end: a report email with a real PDF attachment reaches the - /// SMTP sink, whose DATA carries the multipart structure and the - /// attached bytes (base64-encoded by lettre). - #[cfg(not(target_arch = "wasm32"))] - #[test] - fn a_report_email_with_attachment_reaches_the_sink() { - use lettre::AsyncSmtpTransport; - - let (port, rx) = smtp_sink::start(); - - // A small, real PDF so the attachment is a genuine document. - let trip = crate::email_receipts::TripReceipt { - receipt_id: "BLT-1".into(), - source_email_id: "e1".into(), - date_ms: 1_753_132_800_000, - date_label: "2026-08-17".into(), - amount: 1250.0, - currency: "KES".into(), - pickup: "Westlands".into(), - dropoff: "JKIA".into(), - subject: "Trip receipt".into(), - }; - let report = crate::finance_report::build_trip_report(&[trip], "Trip Expense Report"); - - let rt = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .expect("runtime"); - - rt.block_on(async { - let email = build_report_email( - "jane@example.com", - &["finance@company.com".to_string()], - "Trip expense report — 1 trip(s)", - "Attached is the report.", - &report.bytes, - "trip-expense-report.pdf", - ) - .expect("should build"); - - let mailer = AsyncSmtpTransport::::builder_dangerous("127.0.0.1") - .port(port) - .build(); - send_bounded(&mailer, email, std::time::Duration::from_secs(5)) - .await - .expect("send should succeed"); - }); - - let captured = rx - .recv_timeout(std::time::Duration::from_secs(10)) - .expect("the sink should report the conversation"); - assert_eq!( - captured.rcpt_to, - vec!["RCPT TO:"], - "wrong recipient" - ); - assert!( - captured.data.contains("application/pdf"), - "attachment type missing from DATA" - ); - assert!( - captured.data.contains("trip-expense-report.pdf"), - "attachment filename missing from DATA" - ); - // The plain body and the subject header both land in DATA. - assert!(captured.data.contains("Attached is the report.")); - assert!(captured.data.contains("Trip expense report")); - } - #[cfg(not(target_arch = "wasm32"))] #[test] fn a_real_send_reaches_a_mock_smtp_server() {