diff --git a/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md b/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md new file mode 100644 index 0000000..d7d57d6 --- /dev/null +++ b/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md @@ -0,0 +1,704 @@ +# nigig-email — Brutal Assessment & Execution Plan + +Repository: `https://gitdab.com/andodeki/nigig-org.git` +Crate: `crates/apps/nigig-email` (+ `crates/nigig-core/src/email_worker.rs`) +Assessed at: `2770833` (remote pulled, 61 commits ahead of previous session) +Method: full read of all 11 source files; every claim below was executed or grepped, not inferred. + +--- + +## Verdict + +**Overall: 2.4 / 10.** + +| Dimension | Score | One-line reason | +|---|---|---| +| Architecture | 2 / 10 | No domain model, no receive path, logic inside widgets | +| Performance | 4 / 10 | Small enough not to hurt yet; 10 allocs/event and no pooling are baked in | +| Correctness | 1 / 10 | Binary does not compile; the one advertised feature (bulk) cannot work | +| Design | 2 / 10 | Four byte-identical placeholder pages; shim pile in `lib.rs` | +| Security | 3 / 10 | Password is `Serialize`+`Debug`; no validation. TLS is actually fine. | +| Code quality | 1 / 10 | Zero tests, zero CI, 4 unused deps, dead file, never formatted | + +This is not an email client. It is a **navigation shell with four identical placeholder pages** plus one working SMTP send form bolted onto the tab labelled "Bulk" — which cannot send in bulk. + +The headline findings, all verified: + +| # | Finding | Severity | +|---|---|---| +| **1** | **The binary does not compile.** `main.rs` has unbalanced braces. `cargo check -p nigig-email` fails. | Blocker | +| **2** | **"To (comma-separated)" parses as a single mailbox** — every multi-recipient send fails | Critical | +| **3** | **SMTP password derives `Serialize`** and is POSTed as plaintext JSON on wasm | Critical | +| **4** | **No receive path exists.** No IMAP, no POP3, no fetch. The Inbox cannot show mail. | Critical | +| **5** | **Zero tests.** In the whole crate and in `email_worker.rs`. | Critical | +| **6** | **Zero CI.** No workflow references `nigig-email`. | Critical | +| **7** | **No in-flight guard** — double-tapping Send delivers the email twice | High | +| **8** | Config is rebuilt from 5 text inputs **on every action event** | High | +| **9** | `port.parse().unwrap_or(587)` silently rewrites a typo'd port, changing transport | High | +| **10** | `drafts.rs` (157 lines) is not declared in `mod.rs` — dead code that never compiles | Medium | +| **11** | 3 of 4 dependencies (`serde`, `serde_json`, `chrono`) and `robius-location` are **unused** | Medium | +| **12** | Workspace **violates its own 40-char git-rev CI gate** in 42 places (repo-wide, blocks any green CI here) | Medium | + +One finding I initially rated Critical — TLS validation on port 465 — **is not a +defect**. I checked `lettre`'s source and was wrong; see §1 S1. It is recorded +as Medium (code smell, not vulnerability) rather than deleted, because knowing +which claims survived scrutiny matters more than a tidy list. + +Scale: **1,148 lines** across 11 files, of which roughly **600 are copy-pasted scaffold**. + +--- + +## 0. The crate does not build + +``` +$ cargo check -p nigig-email +error: unexpected closing delimiter: `}` + --> crates/apps/nigig-email/src/main.rs:24:1 +``` + +`cargo check -p nigig-email --lib` → **0 errors**. The library is fine; the **binary target is broken**. + +Look at the nesting in `main.rs`: + +```rust +body +: { + root := mod.widgets.StandaloneFeatureShell { + root_screen := mod.widgets.EmailScreen {} + } // closes StandaloneFeatureShell + standalone_bottom_nav := ... { // ← now a sibling of `root`, at + root_nav := ...ActionBar {} // the wrong nesting depth + } + } // unbalanced from here down +} +``` + +`StandaloneFeatureShell` is closed *before* the bottom nav is declared, so the +brace count never reconciles. The indentation is also self-inconsistent, which +is the visible symptom of a hand-edit that was never compiled. + +**This means nobody has ever run `nigig-email` as a standalone app.** It only +ever gets exercised as a library through `pageflipnav`. The `main.rs` / +`EmailStandaloneApp` / `StandaloneFeatureShell` scaffolding is decorative. + +That single fact reframes everything else in this document: there is no +feedback loop here at all. No CI, no tests, and a binary that cannot start. + +--- + +## 1. Security — the most serious section + +### S1 — MEDIUM: `builder_dangerous` on port 465 is *not* the vulnerability it looks like + +**I initially wrote this up as critical credential exposure. That was wrong, and +I am correcting it rather than quietly dropping it.** + +The code reads alarmingly: + +```rust +465 => { + let tls = TlsParameters::new(config.server.clone())?; + Ok(AsyncSmtpTransport::::builder_dangerous(&config.server) + .port(config.port) + .credentials(creds) + .tls(Tls::Wrapper(tls)) + .build()) +} +``` + +I checked `lettre` 0.11.23's own source instead of trusting the method name. +`relay()` — the "simple and secure" constructor the docs point you to — is +implemented as: + +```rust +pub fn relay(relay: &str) -> Result { + let tls_parameters = TlsParameters::new(relay.into())?; + Ok(Self::builder_dangerous(relay) + .port(SUBMISSIONS_PORT) + .tls(Tls::Wrapper(tls_parameters))) +} +``` + +That is the **same three calls in the same order**. And `TlsParameters::new` +sets: + +```rust +accept_invalid_hostnames: false, +accept_invalid_certs: false, +min_tls_version: TlsVersion::Tlsv12, +``` + +So certificate validation **is** enabled, hostname verification **is** enabled, +and TLS 1.2 is the floor. The genuinely dangerous knobs — +`dangerous_accept_invalid_certs` / `dangerous_accept_invalid_hostnames` — are +never touched. `builder_dangerous` is dangerous only in that it *defaults* to no +TLS; this code immediately supplies `Tls::Wrapper`, which is exactly what +`relay()` does. + +**The real defects here are smaller and different:** + +1. **It reimplements `relay()` by hand.** Functionally equivalent today, but it + silently inherits nothing if `lettre` hardens `relay()` in a future version, + and it reads like a vulnerability to every reviewer — including me. Use + `relay()`. +2. **Port 465 is hardcoded as the only implicit-TLS port.** A provider on 8465 + or any non-standard SMTPS port falls into the `_ =>` STARTTLS arm and fails. +3. **The `_ =>` arm applies STARTTLS to *every* other port**, including 25. + `starttls_relay` does refuse to send credentials if the upgrade fails (the + docs are explicit: *"No credentials or emails will be sent to the server, + protecting from downgrade attacks"*), so this is safe — but it means port 25 + can never work, silently. + +Downgraded from Critical to Medium. The credential-MITM claim was mine, not the +code's. + +### S2 — CRITICAL: the password is a serialisable field + +```rust +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct SmtpConfig { + pub password: String, + ... +} +``` + +Three distinct problems: + +1. **`Serialize` on a secret.** On wasm the entire struct — password included — + is `serde_json::json!`-ed and POSTed to `/api/email`. Every request carries + the plaintext password. If that endpoint logs request bodies (default for + most reverse proxies and APM tools), the password lands in log storage. +2. **`Debug` on a secret.** Any future `log::debug!("{:?}", config)` prints the + password. Nothing does today — I grepped, there are no logging calls — but + the type offers no protection, and this is exactly how credentials leak. + The SMS crate already learned this lesson: `OfflineSmsMessage.body` carries + `#[serde(skip)]` and there is a **CI gate** enforcing it. That precedent + exists in this repo and was not applied here. +3. **`Deserialize`** implies an intent to persist. Today nothing does — the + config lives only in a `#[rust]` widget field and dies with the process. + That is accidentally the safest property of the whole design, and the first + person to add "remember my settings" will write it to plaintext JSON unless + the type stops them. + +The fix is a `Secret`-style newtype with a redacting `Debug`, `#[serde(skip)]` +on serialise, and — when persistence arrives — the platform keystore. The SMS +crate's `SmsScheduleCrypto.java` (AES-256-GCM via `AndroidKeyStore`, fails +closed) is the existing in-repo pattern. + +### S3 — LOW: TLS policy is inherited, not stated + +Also corrected downward. I expected to find no TLS floor; `TlsParameters::new` +sets `min_tls_version: Tlsv12` and `starttls_relay` uses `Tls::Required`, which +aborts before `AUTH` if the upgrade fails. The protection exists. + +What is missing is only that the policy is **implicit**. Nothing in this +repository asserts "email credentials require TLS ≥ 1.2 with a verified +certificate" — it is inherited from a transitive dependency's defaults, so a +`lettre` change or a well-meaning refactor to `builder_dangerous(...)` without +`Tls::Wrapper` would silently remove it with no test failing. + +Fix is a test, not a code change: assert the transport for each port carries +`Tls::Wrapper`/`Tls::Required` and never `Tls::None`. + +### S4 — HIGH: no recipient/header validation → injection surface + +`to`, `subject` and `body` go from `TextInput` straight into +`Message::builder()`. `lettre` does encode headers, so classic CRLF header +injection is mitigated *by the library* — but the application performs no +validation of its own: + +- no length bound on subject or body (a 50 MB paste is attempted verbatim) +- no recipient-count bound (see B1 — a comma list fails anyway) +- no check that `from` matches the authenticated `username`, which is the + single most common cause of silent provider rejection + +This is the same class as SMS **E11** (sender validation), and the same +reasoning applies: what cannot be enforced client-side must at least be +*documented* as a threat, and this crate has no `THREAT_MODEL.md` entry at all. + +### S5 — MEDIUM: wasm endpoint is unauthenticated and unpinned + +```rust +EMAIL_API_URL.get().unwrap_or("/api/email") +``` + +`set_email_api_url` accepts any `String` with no scheme validation. Nothing +calls it today, so the default relative path is used — which is fine. But the +setter permits `http://` and cross-origin absolute URLs, and the POST carries +no auth token, no CSRF token, and no request signing. Any script on the page +can drive the send endpoint using the user's session. + +--- + +## 2. Architecture + +### A1 — CRITICAL: there is no email *model*, and no receive path + +I grepped for `imap`, `pop3`, `fetch`, `receive`, `load_`. Result: **nothing**. + +The crate has: +- an outbound SMTP send (one recipient) +- an SMTP connection test + +That is it. There is no `Email` struct, no `Mailbox`/folder concept, no message +store, no threading, no read/unread state, no attachments, no drafts +persistence, no sync. The "Inbox" page cannot display mail **because no code +in this repository can obtain mail.** + +The crate is named `nigig-email` and declares an Inbox tab. A user tapping +Inbox gets the text *"Top app bar page. Tap below to open a stack screen."* + +### A2 — CRITICAL: business logic is in the widget + +`EmailBulkPage::handle_event` reads five text inputs, parses the port, builds +`SmtpConfig`, and calls the worker. There is no domain layer, so: + +- none of it is testable without a `Cx` +- validation cannot be unit-tested (there is none to test) +- the same logic cannot be reused by the Compose page — which is why Compose + has no send button at all + +Contrast the SMS stack after remediation: `BulkSendRequest::validate()`, +`segment_count()`, `SendPacing`, `SendRateLimiter` all live in the platform +crate precisely so they can be tested on a host with no device. `nigig-email` +has no equivalent seam. + +### A3 — HIGH: the four pages are literally the same file + +I MD5'd each page with its own name normalised away: + +``` +inbox.rs db1fcddf16f6 +compose.rs db1fcddf16f6 ← identical +more.rs db1fcddf16f6 ← identical +drafts.rs 3a3125b45a50 (differs only by a stray blank line) +``` + +Four files, ~630 lines, **one distinct implementation**. Each contains its own +verbatim copy of `push_detail` / `pop_detail` and the same +`"Replace this scaffold with the real workflow for X."` label. + +This is a code generator's output that was never specialised. The SMS +remediation solved this exact problem in Phase F5 by extracting shared page +panes; the same fix applies here and would delete ~450 lines. + +### A4 — MEDIUM: `lib.rs` is a shim pile + +Thirty lines re-exporting `nigig_core` and `nigig_uikit` under local names +(`crate::dir`, `crate::shared`, `crate::persistence`, `crate::features`, +`crate::home`) with the comment *"Compatibility shims for source moved out of +pageflipnav during staged migration."* + +Including a **fake `NavigationBarAction` enum with one variant** declared inline +in `lib.rs` — a UI type defined in a compatibility shim. The migration these +shims serve was never completed. They make every import path in the crate a +lie about where the code actually lives. + +### A5 — MEDIUM: `drafts.rs` is dead + +157 lines, `EmailDraftsPage`, not declared in `pages/mod.rs`. It is **never +compiled**, so it cannot even be known to build. Either wire it up or delete +it; leaving it is worse than both. + +--- + +## 3. Bugs + +### B1 — CRITICAL: multi-recipient send is impossible, and the UI promises it + +The input is labelled: + +```rust +to_input := TextInput { empty_text: "To (comma-separated)" ... } +``` + +The implementation is: + +```rust +let to_mbox: Mailbox = to.parse().map_err(|e| format!("Invalid to: {e}"))?; +``` + +`Mailbox` parses **one** address. Any comma-separated list fails to parse, so +the user gets `Failed: Invalid to: ...` for doing exactly what the placeholder +told them to do. + +So the tab named **"Bulk"** can send to exactly **one** recipient. The crate's +single headline feature does not work. This is a one-line-to-fix defect +(`to.split(',')` → `Vec`, or `Message::builder().to()` per recipient) +that has apparently never been executed once. + +### B2 — HIGH: `SmtpConfig` is rebuilt on every action event + +```rust +if let Event::Actions(actions) = event { + let server = self.text_input(cx, ids!(server_input)).text(); + let port_t = ...text(); // ×5 String allocations + ... + self.smtp_config = SmtpConfig { ... }; // + 4 more via clone() +``` + +`Event::Actions` fires for **every** action in the app — every keystroke, every +scroll, every timer tick from any widget. Each one performs 5 widget lookups, +5 `String` allocations, and 4 more clones building a struct that is only read +when a button is clicked. + +It is also **wrong**, not just wasteful: the config is captured from whatever +the fields happen to contain at the moment an unrelated action fires. Combined +with the port fallback below, this is a config that mutates behind the user. + +### B3 — HIGH: `port.parse().unwrap_or(587)` silently rewrites user input + +```rust +let port: u16 = port_t.parse().unwrap_or(587); +``` + +Type `2525` → works. Type `25 ` with a trailing space, or `465x`, or clear the +field mid-edit → **silently becomes 587**, which then selects the STARTTLS +branch instead of the implicit-TLS branch. The user asked for one transport and +got another, with no message. Combined with S1 the port value selects the +security posture, so a typo silently changes the threat model. + +### B4 — MEDIUM: no in-flight guard; every click spawns another task + +Both `test_btn` and `send_btn` call `spawn_*` unconditionally. Nothing tracks +whether a send is already running. Double-tapping "Send Email" sends the +message **twice** — billed, irreversible, and to a human recipient. + +The SMS crate hit this and fixed it with `BULK_SEND_IN_FLIGHT` (an +`AtomicBool` swap) plus a two-tap confirmation. Neither is present here, and +email has the same irreversibility property. + +### B5 — MEDIUM: no validation before spawning + +Empty server, empty username, empty `from`, empty recipient, empty body — all +accepted. The worker spawns, the network call fails, and the user sees a raw +`lettre` error string. Every one of these is cheap to check locally with a +clear message. + +### B6 — LOW: `EmailWorkerAction::None` exists only to satisfy a trait + +`ActionDefaultRef` needs a default, so the enum carries a `None` variant that +is never constructed or matched. It widens every `match` for no behaviour. + +--- + +## 4. Performance + +### P1 — HIGH: per-event allocations in `handle_event` (see B2) + +Ten heap allocations per action event, discarded unread. On a page with a +`TextInput` focused, that is per keystroke. + +### P2 — MEDIUM: `CachedWidget` on all four pages, none with content + +```rust +inbox_page := View { CachedWidget { inbox_page_inner := ... } } +compose_page := View { CachedWidget { ... } } +bulk_page := View { CachedWidget { ... } } +more_page := View { CachedWidget { ... } } +``` + +All four pages are instantiated and cached for the process lifetime. Today they +are placeholders, so the cost is small — but this locks in a design where the +inbox (the page that will eventually hold a scrollable list of thousands of +messages) can never be released. Worth deciding deliberately rather than by +default. + +### P3 — MEDIUM: no connection pooling used + +`lettre` is built with the `pool` feature, but `build_transport` constructs a +**brand-new transport per operation**. Every send performs a fresh TCP +handshake, TLS negotiation and `AUTH`. For a bulk feature — the crate's stated +purpose — that is the dominant cost and the fastest way to get rate-limited or +flagged as abusive by the provider. + +### P4 — LOW: fire-and-forget tasks, no cancellation, no explicit timeout + +`crate::platform::spawn` is `tokio::spawn` on native and `spawn_local` on wasm. +Nothing retains the `JoinHandle`, so no operation can be cancelled. + +`lettre` does apply a **60-second default** per SMTP command, so a black-holed +server does eventually fail rather than hanging forever — I checked, this is not +the unbounded hang I first assumed. But the application sets no timeout of its +own, so the user watches `"Testing..."` for a full minute with no way to abort, +and a `send` that spans several commands can exceed that. + +Combined with **B4** (no in-flight guard), a user who taps Send during those 60 +seconds queues a second delivery. + +--- + +## 5. Code quality + +### Q1 — CRITICAL: zero tests + +``` +$ grep -rc '#\[test\]' crates/apps/nigig-email/src/ → 0 +$ grep -c '#\[test\]' crates/nigig-core/src/email_worker.rs → 0 +``` + +Not one test in 1,148 lines of application code plus 210 lines of worker. +Nothing verifies port→transport selection, address parsing, or error mapping — +all of which are pure functions that need no network. + +### Q2 — CRITICAL: zero CI + +No workflow in `.forgejo/workflows/` mentions `nigig-email`. It is not built, +not linted, not tested by any automated process. That is precisely how a +**binary with unbalanced braces** reached `main` and stayed there. + +### Q3 — HIGH: three unused dependencies + one unused platform dep + +``` +serde → 0 files reference it +serde_json → 0 +chrono → 0 +robius-location → 0 (declared, never imported) +``` + +`robius-location` is the *identical* defect that SMS Phase B removed from +`nigig-build`, `nigig-core` and `nigig-uikit` — it dragged in RUSTSEC +exemptions and an LGPL-2.1 question for a dependency that was never called. +A CI gate was even added to stop it coming back: + +```yaml +- name: The removed platform deps must not come back +``` + +That gate covers `nigig-build`, `nigig-core`, `nigig-uikit` — **not** +`nigig-email`. So the same mistake sits here, un-gated. + +### Q4 — HIGH: the workspace violates its own supply-chain gate *right now* + +`nigig-build.yml` contains a gate requiring **full 40-character** git revs, +added deliberately in `5e71457` with a comment explaining that abbreviated revs +become ambiguous as a repo grows. I ran it: + +``` +$ grep -rn 'rev = ' --include=Cargo.toml . | grep -vE 'rev = "[0-9a-f]{40}"' +./crates/apps/geohot/Cargo.toml:7: ... rev = "5efe6e24c" +./crates/apps/map/Cargo.toml:8: ... rev = "5efe6e24c" +>>> GATE FAILS +``` + +**42 declarations across 34 crates** use the 9-character rev `5efe6e24c`, +introduced by `9d647ce` ("bump makepad fork rev"). That commit **broke the gate +that `5e71457` created**. `nigig-email` is one of the 34. + +This is repo-wide, not email-specific, but it lands in the plan because the +email crate cannot be given a green CI job while a shared gate is red. + +### Q5 — MEDIUM: no docs, no `README`, no module comments + +Not one `//!` module doc. The only comments are the copy-pasted +*"First forward the event so dynamic StackNavigation children can produce +actions"* (×4) and the *"Compatibility shims"* note. No explanation of the +SMTP threat model, the wasm/native split, or why `builder_dangerous` was chosen. + +### Q6 — MEDIUM: `rustfmt` never run + +The crate is not in any fmt gate. Mixed indentation in `main.rs` is what made +the brace bug invisible to review. + +--- + +## 6. Execution plan + +Ordered by **severity × cost**. Every phase ends green on real CI, on a +registered runner, and is pushed before the next begins. + +Principles carried over from the SMS remediation, which worked: +- **Negative-test every gate** — revert the fix, confirm the gate fails. +- **Ratchet, don't blanket-disable.** A step that always fails gets ignored. +- **Host-testable seams first.** CI runs on Linux; SMTP and Android are stubs. +- State plainly what is **not** device- or network-verified. + +--- + +### Progress log + +Updated as work lands, so this document stays a live plan rather than a +snapshot. Commits are on `main`. + +| Commit | What | +|---|---| +| `28d0608` | **Phase C2 (early)** — `email_account.rs` + `email_store.rs`: session state, account validation, sender-thread grouping, char-safe previews. 38 host tests. | +| `1ea9ad6` | **Phase 0.1 DONE** — `main.rs` braces fixed; the binary compiles for the first time. | +| `50760e0` | **Feature: account-gated inbox** — sender list + thread reader reusing `nigig_uikit::shared::conversation`; setup form moved off the Bulk tab; `drafts.rs` deleted (0.4). | + +Verified: `cargo check -p nigig-email --all-targets` → 0 errors; +41 tests pass (38 core + 3 UI helpers); `nigig-email` clippy down to 2 +pre-existing `unexpected_cfgs` from the `app_main!` macro; `cargo fmt` +clean for this crate. + +**Caveat on verification base:** `origin/main` at `86c9595` does not +resolve — the makepad bump dropped the `maps` feature `pageflipnav` +requires. Pre-existing and unrelated; confirmed by stashing all my +changes and reproducing on a pristine tree. Numbers above are from +`2faadb7`, the commit before that bump. + +--- + +### The requested feature, folded into the phases + +The ask: *an inbox list like SMS when signed in; tap a sender to read the +thread with the same transition; the connection form when signed out.* + +That is not a separate workstream — it is Phase C (make it an email +client) pulled forward, with the parts that need no receive path done +first. Mapping: + +| Ask | Phase | State | +|---|---|---| +| Domain model for messages/threads | C2 | **done** (`28d0608`) | +| Inbox list of senders | C4 | **done** (`50760e0`) | +| Thread reader with SMS-style transition | C4 | **done** (`50760e0`) | +| Signed-out → connection form | new, A-adjacent | **done** (`50760e0`) | +| Real mail in the list | **C1 + C4** | **blocked on your C1 decision** | + +What is real today: the gate, the list, the grouping, the transition, the +unread handling, the setup form and its validation. What is not: the +*contents*. The list renders `email_store::sample_thread()` because no +receive path exists. Everything above it is the code a real fetch will +populate without further UI change. + +--- + +### Phase 0 — Make it verifiable (blocker) + +Nothing else can be trusted until the crate builds and something runs it. + +| ID | Task | +|---|---| +| ~~0.1~~ | ~~**Fix `main.rs` braces.**~~ **DONE** (`1ea9ad6`) — was missing the `StandaloneFeatureBody` wrapper; `--lib` had always passed, so only the binary was broken. | +| 0.2 | **Repair the 40-char rev gate repo-wide.** Resolve `5efe6e24c` → full SHA, rewrite all 42 declarations. This is mechanical and must land in its own commit. | +| 0.3 | **Create `.forgejo/workflows/email.yml`**: `gates` (source scans), `nigig-email` (check + clippy + test), `supply-chain` (`cargo deny`, lockfile, unused deps). | +| ~~0.4~~ | ~~**Delete or wire `drafts.rs`.**~~ **DONE** (`50760e0`) — deleted. 157 lines never declared in `pages/mod.rs`, so never compiled. | +| 0.5 | **Drop unused deps**: `serde_json`, `robius-location`. *Revised:* `chrono` is now used (`format_thread_time`) and `serde` is used by `email_account`/`email_store`, so only two remain. Extend the "removed platform deps must not come back" gate to cover `nigig-email`. | +| 0.7 | **NEW — unblock `origin/main`.** The makepad bump `86c9595` dropped the `maps` feature `pageflipnav` declares, so the workspace does not resolve at HEAD. Nothing can be CI-verified until this is fixed. Higher priority than 0.2. | +| 0.6 | **Add `nigig-email` to a fmt gate**, or record explicitly why not (SMS chose report-only; email is small enough to just format). | + +Exit: `cargo check`/`clippy`/`test -p nigig-email` green on a real runner. + +--- + +### Phase A — Security (critical) + +| ID | Task | +|---|---| +| A1 | **`Secret` newtype for the password.** Redacting `Debug` (`"***"`), `#[serde(skip)]`. **CI gate**: the password field must never be plainly serialisable — mirrors the SMS `#[serde(skip)]` body gate. *This is the real critical item; do it first.* | +| A2 | **Replace the hand-rolled 465 branch with `relay()`.** Behaviour-preserving today (verified equivalent), but stops the code reading as a vulnerability and inherits future hardening. Add a test asserting transport choice per port. | +| A3 | **Pin the TLS policy with a test**, so the inherited defaults cannot be silently removed: assert `Tls::Wrapper`/`Tls::Required` per port and never `Tls::None`. | +| A4 | **Validate before dispatch**: non-empty server/username/from, `from` parses, bounded subject (≤998 bytes per RFC 5322) and body, recipient count cap. All host-testable. | +| A5 | **Scheme-check `set_email_api_url`**; reject non-HTTPS absolute URLs. Document that the wasm endpoint needs auth. | +| A6 | **Add a `THREAT_MODEL.md` section** for email: what the client can enforce, and what it cannot (`from` spoofing is server-side, per SMS E11). | + +Negative tests: remove `#[serde(skip)]` → A1 gate fails. Set +`Tls::None` on either branch → A3 test fails. + +--- + +### Phase B — Fix the feature that is advertised (critical) + +| ID | Task | +|---|---| +| B1 | **Multi-recipient send.** Parse the comma list into `Vec`, report per-recipient success/failure. This is what "Bulk" claims to do. Reuse the SMS `recipient_csv.rs` normalisation approach for splitting/dedupe. | +| B2 | **`EmailSendRequest` domain type** in `nigig-core` with `validate()` — the seam that makes everything above testable. Mirrors `BulkSendRequest`. | +| B3 | **Move config assembly out of `handle_event`.** Read inputs on `.changed()` only, or read once at click time. Kills 10 allocations/event. | +| B4 | **Strict port parsing.** Empty → default *with a visible note*; invalid → refuse and say so. Never silently rewrite. | +| B5 | **In-flight guard + two-tap confirmation** before spending real sends (SMS A7/D5 pattern). | +| B6 | **Timeout + cancel** on SMTP operations. No unbounded `"Testing..."`. | + +--- + +### Phase C — Make it an email client (large; scope decision needed) + +This is the phase that determines whether `nigig-email` is a product or a +send-only form. **It needs a product decision before any code.** + +| ID | Task | +|---|---| +| **C1** | **DECIDE THE RECEIVE STRATEGY — the one blocking question.** IMAP on device (`async-imap`) vs a server-side proxy API. See the decision note below. **Everything else in this phase is either done or waits on this.** | +| ~~C2~~ | ~~**Define the domain model.**~~ **DONE** (`28d0608`) — `EmailMessage`, `EmailThreadSummary`, grouping, previews, filtering. 38 tests. `Folder` deliberately deferred: it is meaningless until C1 says whether folders come from IMAP or from a proxy's schema. | +| C3 | **Persistence** — reuse `nigig-core::persistence`; bodies encrypted at rest, per SMS E1/E3. Note `EmailMessage.body` is *not* `#[serde(skip)]`-ed the way `OfflineSmsMessage.body` is, because a mail cache that drops bodies is useless — so encryption is mandatory here, not optional. | +| ~~C4a~~ | ~~**Inbox list + thread reader.**~~ **DONE** (`50760e0`) — sender list, thread push/pop, unread handling, signed-out gate. | +| C4b | **Wire it to real data.** Replace `sample_thread()` with the C1 fetch. Add loading / error / empty states — currently only empty exists. | +| C5 | **Give Compose a send button** wired to B2, replacing the scaffold. | +| C6 | **Bulk pacing.** Email providers rate-limit harder than carriers. Port `SendPacing` from `robius-sms` — already generic arithmetic. | +| C7 | **NEW — pull-to-refresh + background fetch** once C1 lands. The SMS crate's D1/D2 pattern (worker thread → results queue → `SignalToUI` → drained on the UI thread) applies directly; do not fetch from `draw_walk`. | + +#### C1 — the decision I need from you + +| | IMAP on device | Server-side proxy | +|---|---|---| +| Credentials | Stay on the phone, but stored there — needs keystore work (A1) | Never touch the client after setup | +| Offline | Works | Needs a local cache anyway | +| Effort | `async-imap` + TLS + parsing + sync state | Client is a thin HTTP client; server is new infrastructure | +| wasm | IMAP over raw TCP is impossible in a browser — needs a proxy *anyway* | Same code path on every platform | +| Resolves S2? | No — you still hold the password | **Largely yes** | + +My read: **the proxy wins on merit**, mostly because the wasm target +already forces one and because it retires the password-storage problem +rather than mitigating it. But it is infrastructure you may not want to +run, and IMAP-on-device is the only option that works with no backend at +all. This is a product call, not a technical one, which is why I have not +made it. + +--- + +### Phase D — Design & duplication + +| ID | Task | +|---|---| +| D1 | **Extract the shared page scaffold.** One parameterised pane replaces 4 copies (~450 lines deleted). SMS Phase F5 precedent. | +| D2 | **Delete the `lib.rs` shims** or finish the migration they were staged for. A UI enum declared in a compat shim is not acceptable long-term. | +| D3 | **Reconsider `CachedWidget` on the inbox** once it holds a real list. | +| D4 | **Connection pooling** — actually use the `pool` feature already compiled in. | +| D5 | Drop `EmailWorkerAction::None` if `ActionDefaultRef` can be satisfied otherwise. | + +--- + +### Phase E — Tests & ratchets + +| ID | Task | +|---|---| +| E1 | **Unit-test the pure logic**: port→transport, address parsing/splitting, validation, error mapping. Target ≥40 tests; these need no network. | +| E2 | **Property-test address parsing** (proptest is already a dev-dep in SMS) — never panic on arbitrary input. | +| E3 | **Test-count floor gate**, as SMS has (`FLOOR=100`). | +| E4 | **Clippy ratchet** at the measured baseline. | +| E5 | **Integration test** against a local SMTP sink (e.g. a `MockSmtp` listener on 127.0.0.1) — verifies the transport path without a real provider. | + +--- + +## 7. Sequencing note + +Phase 0 is non-negotiable and is roughly a day. Phases A and B are each +small in code and large in value — A1 is one line, B1 is a few. Together they +turn a crate that cannot build into one that securely sends to multiple +recipients, with tests. + +**Phase C is the real question.** Everything before it is repair; C is +construction, and it is where most of the remaining effort lives. I would not +start C without an explicit answer on C1 (IMAP-on-device vs server proxy), +because that choice changes the security model, the dependency set, and the +persistence design. Choosing "server proxy" also happens to resolve S1/S2 more +convincingly than any client-side fix can. + +## 8. What I have not verified + +Stated plainly, because the point of this document is to be trusted: + +- **No SMTP path has been executed.** No live server, no MITM test. The S1/S3 + analysis is a read of `lettre` 0.11.23's vendored source (`relay()`, + `TlsParameters::new`, `TlsVersion`), which is why I was able to catch my own + error — but it is still a read, not an observed handshake. +- **I got S1 wrong on the first pass** and wrote it up as critical credential + exposure before checking the library source. The corrected entry is in §1. + Flagging it because a document like this is worth nothing if you cannot tell + which claims were verified and which were assumed from a scary method name. +- **The wasm path has not been built.** `call_email_api` is + `#[cfg(target_arch = "wasm32")]`; I compiled for host only. +- **B1 is a code read, not a run** — the binary does not compile, so the + multi-recipient failure is deduced from `Mailbox::parse`'s single-address + contract rather than observed at runtime. It is unambiguous, but it is a read. +- **Test/clippy baselines are not yet measured** for the ratchets in Phase E; + they must be captured from a real run, not guessed. diff --git a/crates/apps/nigig-email/src/email_frame/action_bars.rs b/crates/apps/nigig-email/src/email_frame/action_bars.rs index b058184..377da5f 100644 --- a/crates/apps/nigig-email/src/email_frame/action_bars.rs +++ b/crates/apps/nigig-email/src/email_frame/action_bars.rs @@ -1,9 +1,6 @@ use crate::features::action_page_navigation::ActionPageNavigationAction; use crate::home::navigation_tab_bar::NavigationBarAction; -use crate::shared::{ - navigation_bar_button::{NavigationBarButton, NavigationBarButtonWidgetExt}, - styles::*, -}; +use crate::shared::navigation_bar_button::NavigationBarButtonWidgetExt; use makepad_widgets::*; const DOUBLE_TAP_HOME_SECS: f64 = 0.55; diff --git a/crates/apps/nigig-email/src/email_frame/pages/account_setup.rs b/crates/apps/nigig-email/src/email_frame/pages/account_setup.rs new file mode 100644 index 0000000..922cc6d --- /dev/null +++ b/crates/apps/nigig-email/src/email_frame/pages/account_setup.rs @@ -0,0 +1,247 @@ +// Account setup form, shown when no email account is connected. +// +// This is the "not logged in" half of the Inbox page. It is the form that +// used to live on the Bulk tab -- SMTP server, port, username, password, +// from -- moved here, because that is where it belongs: you connect an +// account once, then you read mail. Leaving it on a tab called "Bulk" +// meant the app had a credentials form and no way to see your inbox. +// +// All validation goes through nigig_core::email_account::AccountDraft, +// which is unit tested on the host. This widget only moves strings. + +use makepad_widgets::*; +use nigig_core::email_account::{guess_provider, AccountDraft}; + +script_mod! { + use mod.prelude.widgets.* + use mod.widgets.* + + mod.widgets.EmailAccountSetup = #(EmailAccountSetup::register_widget(vm)) { + width: Fill, height: Fill + flow: Down + + setup_scroll := ScrollYView { + width: Fill, height: Fill + flow: Down + padding: Inset{left: 18, right: 18, top: 12, bottom: 22} + spacing: 12 + + intro_card := RoundedView { + width: Fill, height: Fit + flow: Down + spacing: 6 + padding: 18 + show_bg: true + draw_bg +: { color: #xF8FAFC, border_radius: 22.0, border_size: 1.0, border_color: #xE2E8F0 } + + Label { + text: "Connect your email" + draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 22.0 } } + } + Label { + width: Fill, height: Fit + text: "Add an account to read your inbox and send mail. Your password stays on this device for the current session only." + draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 11.0 } } + } + } + + form_card := RoundedView { + width: Fill, height: Fit + flow: Down + spacing: 8 + padding: 18 + show_bg: true + draw_bg +: { color: #xFFFFFF, border_radius: 22.0, border_size: 1.0, border_color: #xE2E8F0 } + + Label { text: "Email address" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + address_input := TextInput { + width: Fill, height: 42 + empty_text: "you@example.com" + draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } + } + + Label { text: "SMTP server" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + server_input := TextInput { + width: Fill, height: 42 + empty_text: "smtp.example.com" + draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } + } + + Label { text: "Port" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + port_input := TextInput { + width: Fill, height: 42 + text: "587" + empty_text: "587" + draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } + } + + Label { text: "Username" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + username_input := TextInput { + width: Fill, height: 42 + empty_text: "Usually your email address" + draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } + } + + Label { text: "Password or app password" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + password_input := TextInput { + width: Fill, height: 42 + empty_text: "••••••••" + is_password: true + draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } + } + + Label { text: "Display name (optional)" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + display_name_input := TextInput { + width: Fill, height: 42 + empty_text: "Shown to people you email" + draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } + } + + connect_btn := Button { + width: Fill, height: 48 + text: "Connect account" + draw_bg +: { color: #x1C274C, color_hover: #x2A3F6E, border_radius: 14.0 } + draw_text +: { color: #xFFFFFF, text_style: theme.font_bold { font_size: 14.0 } } + } + + setup_status := Label { + width: Fill, height: Fit + text: "" + draw_text +: { color: #xB4232C, text_style: theme.font_regular { font_size: 10.5 } } + } + } + } + } +} + +/// Emitted when the user asks to connect. Carries the raw draft; the +/// screen validates and drives the connection test. +#[derive(Clone, Debug, Default)] +pub enum EmailAccountSetupAction { + #[default] + None, + Connect(AccountDraft), +} + +impl ActionDefaultRef for EmailAccountSetupAction { + fn default_ref() -> &'static Self { + static DEFAULT: EmailAccountSetupAction = EmailAccountSetupAction::None; + &DEFAULT + } +} + +#[derive(Script, ScriptHook, Widget)] +pub struct EmailAccountSetup { + #[deref] + view: View, + /// Set when the address field last auto-filled the server, so a user + /// who typed their own server is never overwritten. + #[rust] + autofilled_server: bool, +} + +impl Widget for EmailAccountSetup { + fn handle_event(&mut self, cx: &mut Cx, event: &Event, scope: &mut Scope) { + self.view.handle_event(cx, event, scope); + + let Event::Actions(actions) = event else { + return; + }; + + // Autofill the server for known providers as soon as the address + // has a recognisable domain. This is why the form is not five + // mandatory fields for a Gmail user. + if self + .view + .text_input(cx, ids!(address_input)) + .changed(actions) + .is_some() + { + let address = self.view.text_input(cx, ids!(address_input)).text(); + if let Some((server, port)) = guess_provider(&address) { + let current = self.view.text_input(cx, ids!(server_input)).text(); + // Only fill a blank field, or one we filled ourselves. + if current.trim().is_empty() || self.autofilled_server { + self.view + .text_input(cx, ids!(server_input)) + .set_text(cx, server); + self.view + .text_input(cx, ids!(port_input)) + .set_text(cx, &port.to_string()); + self.autofilled_server = true; + self.view.redraw(cx); + } + } + } + + if self.view.button(cx, ids!(connect_btn)).clicked(actions) { + let draft = self.read_draft(cx); + // Validate here so the form can report every bad field at + // once; the screen re-validates before touching the network. + match draft.validate() { + Err(errors) => { + let msg = errors + .iter() + .map(|e| e.message()) + .collect::>() + .join(" "); + self.view.label(cx, ids!(setup_status)).set_text(cx, &msg); + self.view.redraw(cx); + } + Ok(_) => { + self.view + .label(cx, ids!(setup_status)) + .set_text(cx, "Checking connection…"); + self.view.redraw(cx); + cx.widget_action(self.widget_uid(), EmailAccountSetupAction::Connect(draft)); + } + } + } + } + + fn draw_walk(&mut self, cx: &mut Cx2d, scope: &mut Scope, walk: Walk) -> DrawStep { + self.view.draw_walk(cx, scope, walk) + } +} + +impl EmailAccountSetup { + fn read_draft(&mut self, cx: &mut Cx) -> AccountDraft { + AccountDraft { + address: self.view.text_input(cx, ids!(address_input)).text(), + smtp_server: self.view.text_input(cx, ids!(server_input)).text(), + smtp_port: self.view.text_input(cx, ids!(port_input)).text(), + username: self.view.text_input(cx, ids!(username_input)).text(), + password: self.view.text_input(cx, ids!(password_input)).text(), + display_name: self.view.text_input(cx, ids!(display_name_input)).text(), + } + } + + /// Show why a connection failed, and keep what the user typed. + pub fn show_error(&mut self, cx: &mut Cx, message: &str) { + self.view + .label(cx, ids!(setup_status)) + .set_text(cx, message); + self.view.redraw(cx); + } + + /// Prefill from a previously-entered account after a failure, so the + /// user fixes one field instead of retyping six. + pub fn prefill(&mut self, cx: &mut Cx, account: &nigig_core::email_account::EmailAccount) { + self.view + .text_input(cx, ids!(address_input)) + .set_text(cx, &account.address); + self.view + .text_input(cx, ids!(server_input)) + .set_text(cx, &account.smtp_server); + self.view + .text_input(cx, ids!(port_input)) + .set_text(cx, &account.smtp_port.to_string()); + self.view + .text_input(cx, ids!(username_input)) + .set_text(cx, &account.username); + self.view + .text_input(cx, ids!(display_name_input)) + .set_text(cx, &account.display_name); + self.view.redraw(cx); + } +} diff --git a/crates/apps/nigig-email/src/email_frame/pages/drafts.rs b/crates/apps/nigig-email/src/email_frame/pages/drafts.rs deleted file mode 100644 index ca017d6..0000000 --- a/crates/apps/nigig-email/src/email_frame/pages/drafts.rs +++ /dev/null @@ -1,157 +0,0 @@ - -use makepad_widgets::*; -use crate::shared::context_nav_action::ContextNavAction; - -script_mod! { - use mod.prelude.widgets.* - use mod.widgets.* - - mod.widgets.EmailDraftsPage = #(EmailDraftsPage::register_widget(vm)) { - width: Fill, height: Fill - - page_stack := StackNavigation { - root_view +: { - width: Fill, height: Fill - flow: Down - - page_top_bar := SolidView { - width: Fill, height: 52 - flow: Right - align: Align{y: 0.5} - padding: Inset{left: 16, right: 16, top: 0, bottom: 0} - show_bg: true - draw_bg +: { color: #xFFFFFF } - - Label { - width: Fill, height: Fit - text: "Drafts" - draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 20.0 } } - } - } - - page_body := ScrollYView { - width: Fill, height: Fill - flow: Down - padding: Inset{left: 18, right: 18, top: 12, bottom: 22} - spacing: 12 - - RoundedView { - width: Fill, height: Fit - flow: Down - spacing: 8 - padding: 18 - show_bg: true - draw_bg +: { color: #xF8FAFC, border_radius: 22.0, border_size: 1.0, border_color: #xE2E8F0 } - Label { text: "Drafts" draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 22.0 } } } - Label { width: Fill, height: Fit, text: "Top app bar page. Tap below to open a stack screen with RobrixStackNavigationView back navigation." draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 11.0 } } } - } - - open_detail_btn := Button { - width: Fill, height: 54 - text: "Open Drafts workflow" - draw_bg +: { color: #xEEF4FF, color_hover: #xDDEBFF, color_down: #xCFE2FF, border_radius: 16.0, border_size: 1.0, border_color: #xD7E5FF } - draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 12.0 } } - } - } - } - - stack_templates: { - EmailDraftsPageDetailStackView := mod.widgets.RobrixStackNavigationView { - body +: { - detail_body := ScrollYView { - width: Fill, height: Fill - flow: Down - padding: Inset{left: 18, right: 18, top: 18, bottom: 22} - spacing: 12 - - Label { - width: Fill, height: Fit - text: "Drafts details" - draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 22.0 } } - } - Label { - width: Fill, height: Fit - text: "This is a RobrixStackNavigationView destination. The built-in header above supplies the title and back arrow, just like SMS conversation screens." - draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 11.0 } } - } - RoundedView { - width: Fill, height: Fit - flow: Down - padding: 16 - spacing: 8 - show_bg: true - draw_bg +: { color: #xF8FAFC, border_radius: 18.0, border_size: 1.0, border_color: #xE2E8F0 } - Label { text: "Next screen content" draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 13.0 } } } - Label { width: Fill, height: Fit, text: "Replace this scaffold with the real workflow for Drafts." draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } - } - } - } - } - } - } - } -} - -#[derive(Script, ScriptHook, Widget)] -pub struct EmailDraftsPage { - #[deref] - view: View, - #[rust] - current_detail_view: Option, -} - -impl Widget for EmailDraftsPage { - fn handle_event(&mut self, cx: &mut Cx, event: &Event, scope: &mut Scope) { - // First forward the event so dynamic StackNavigation children can produce actions. - // Then handle the Event::Actions carried by this turn. This follows the SMS/Home pattern. - self.view.handle_event(cx, event, scope); - - if let Event::Actions(actions) = event { - if self.view.button(cx, ids!(open_detail_btn)).clicked(actions) { - self.push_detail(cx); - } - - for action in actions { - if let StackNavigationTransitionAction::ViewReleased(view_id) = action.as_widget_action().cast() { - if self.current_detail_view == Some(view_id) { - self.current_detail_view = None; - } - } - if let StackNavigationAction::Pop = action.as_widget_action().cast() { - self.pop_detail(cx); - } - } - } - } - - fn draw_walk(&mut self, cx: &mut Cx2d, scope: &mut Scope, walk: Walk) -> DrawStep { - self.view.draw_walk(cx, scope, walk) - } -} - -impl EmailDraftsPage { - fn push_detail(&mut self, cx: &mut Cx) { - let stack = self.view.stack_navigation(cx, ids!(page_stack)); - if stack.is_transitioning() { return; } - if let Some((view_id, _view)) = stack.create_view_from_template(cx, id!(EmailDraftsPageDetailStackView)) { - self.current_detail_view = Some(view_id); - stack.set_title(cx, view_id, "Drafts"); - stack.push(cx, view_id); - cx.action(ContextNavAction::HideBottomNav); - self.view.redraw(cx); - } - } - - fn pop_detail(&mut self, cx: &mut Cx) { - let stack = self.view.stack_navigation(cx, ids!(page_stack)); - if stack.is_transitioning() { return; } - self.current_detail_view = None; - stack.pop_to_root(cx); - cx.action(ContextNavAction::ShowBottomNav); - self.view.redraw(cx); - } -} - - - - diff --git a/crates/apps/nigig-email/src/email_frame/pages/inbox.rs b/crates/apps/nigig-email/src/email_frame/pages/inbox.rs index c79aaa1..6b4c1b9 100644 --- a/crates/apps/nigig-email/src/email_frame/pages/inbox.rs +++ b/crates/apps/nigig-email/src/email_frame/pages/inbox.rs @@ -1,5 +1,38 @@ -use crate::shared::context_nav_action::ContextNavAction; +// Email inbox: a list of senders, and a thread per sender. +// +// Structure mirrors the SMS inbox deliberately, because the interaction +// is the same and users move between the two features: +// +// PortalList of preview rows +// -> tap a row +// -> SharedConversationPreviewAction::Clicked +// -> push a RobrixStackNavigationView with the thread timeline +// -> back arrow pops to the list +// +// It reuses `nigig_uikit::shared::conversation`, which already exists for +// exactly this: `SharedConversationKind::Email` is a variant in that +// module's `types.rs`, and the row widget, message bubbles and date +// dividers are all generic. Nothing here reimplements what SMS has. +// +// The page is gated on account state. Signed out, it shows the setup +// form; signed in, the list. Before this, the Inbox tab rendered the text +// "Top app bar page. Tap below to open a stack screen." -- a placeholder +// with no path to any mail, and the credentials form was on the Bulk tab. + use makepad_widgets::*; +use nigig_core::email_account::{AccountDraft, EmailAccount, SessionState}; +use nigig_core::email_store::{ + group_by_sender, sample_thread, thread_for_sender, total_unread, EmailMessage, + EmailThreadSummary, +}; +use nigig_core::email_worker::{spawn_smtp_test, EmailWorkerAction, SmtpConfig}; +use nigig_uikit::shared::conversation::conversation_preview::{ + SharedConversationPreviewAction, SharedConversationPreviewProps, +}; + +use crate::shared::context_nav_action::ContextNavAction; + +use super::account_setup::{EmailAccountSetupAction, EmailAccountSetupWidgetExt}; script_mod! { use mod.prelude.widgets.* @@ -21,67 +54,87 @@ script_mod! { show_bg: true draw_bg +: { color: #xFFFFFF } - Label { + inbox_title := Label { width: Fill, height: Fit text: "Inbox" draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 20.0 } } } + unread_badge := Label { + width: Fit, height: Fit + text: "" + draw_text +: { color: #x1a73e8, text_style: theme.font_bold { font_size: 12.0 } } + } } - page_body := ScrollYView { + // Signed out vs signed in. Only one is ever visible. + inbox_body_flip := PageFlip { width: Fill, height: Fill - flow: Down - padding: Inset{left: 18, right: 18, top: 12, bottom: 22} - spacing: 12 + active_page: @signed_out_page - RoundedView { - width: Fill, height: Fit - flow: Down - spacing: 8 - padding: 18 - show_bg: true - draw_bg +: { color: #xF8FAFC, border_radius: 22.0, border_size: 1.0, border_color: #xE2E8F0 } - Label { text: "Inbox" draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 22.0 } } } - Label { width: Fill, height: Fit, text: "Top app bar page. Tap below to open a stack screen with RobrixStackNavigationView back navigation." draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 11.0 } } } + signed_out_page := View { + width: Fill, height: Fill + account_setup := mod.widgets.EmailAccountSetup {} } - open_detail_btn := Button { - width: Fill, height: 54 - text: "Open Inbox workflow" - draw_bg +: { color: #xEEF4FF, color_hover: #xDDEBFF, color_down: #xCFE2FF, border_radius: 16.0, border_size: 1.0, border_color: #xD7E5FF } - draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 12.0 } } + signed_in_page := View { + width: Fill, height: Fill + flow: Down + + threads_list := PortalList { + keep_invisible: false + auto_tail: false + width: Fill, height: Fill + flow: Down + spacing: 0.0 + + conversation_preview := mod.widgets.SharedConversationPreview {} + empty_state := View { + width: Fill, height: 220 + flow: Down + align: Align{x: 0.5, y: 0.5} + spacing: 8 + Label { + text: "No mail yet" + draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 15.0 } } + } + Label { + width: 260, height: Fit + text: "Messages from your account will appear here." + draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 11.0 } } + } + } + bottom_filler := View { width: Fill, height: 80.0 } + } } } } stack_templates: { - EmailInboxPageDetailStackView := mod.widgets.RobrixStackNavigationView { + // The thread screen. Same shell SMS conversations use, so + // the header, back arrow and transition behave identically. + EmailThreadStackView := mod.widgets.RobrixStackNavigationView { body +: { - detail_body := ScrollYView { + thread_body := View { width: Fill, height: Fill flow: Down - padding: Inset{left: 18, right: 18, top: 18, bottom: 22} - spacing: 12 - Label { + thread_subject := Label { width: Fill, height: Fit - text: "Inbox details" - draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 22.0 } } + padding: Inset{left: 18, right: 18, top: 12, bottom: 4} + text: "" + draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 15.0 } } } - Label { - width: Fill, height: Fit - text: "This is a RobrixStackNavigationView destination. The built-in header above supplies the title and back arrow, just like SMS conversation screens." - draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 11.0 } } - } - RoundedView { - width: Fill, height: Fit + + thread_timeline := PortalList { + keep_invisible: false + auto_tail: true + width: Fill, height: Fill flow: Down - padding: 16 - spacing: 8 - show_bg: true - draw_bg +: { color: #xF8FAFC, border_radius: 18.0, border_size: 1.0, border_color: #xE2E8F0 } - Label { text: "Next screen content" draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 13.0 } } } - Label { width: Fill, height: Fit, text: "Replace this scaffold with the real workflow for Inbox." draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + spacing: 0.0 + + sent_message := mod.widgets.SharedSentMessageBubble {} + received_message := mod.widgets.SharedReceivedMessageBubble {} + thread_filler := View { width: Fill, height: 40.0 } } } } @@ -95,66 +148,350 @@ script_mod! { pub struct EmailInboxPage { #[deref] view: View, + #[rust] - current_detail_view: Option, + session: SessionState, + /// Session-only. Never written to disk -- see the note in + /// `nigig_core::email_account`: `EmailAccount` is the persistable half + /// and deliberately has no password field. + #[rust] + password: String, + /// The draft awaiting a connection result, so a failure can report + /// against the account the user actually typed. + #[rust] + pending: Option, + + #[rust] + messages: Vec, + #[rust] + threads: Vec, + + #[rust] + current_thread_view: Option, + /// Sender whose thread is open, so the timeline can be rebuilt if new + /// mail arrives while it is on screen. + #[rust] + open_sender: Option, } impl Widget for EmailInboxPage { fn handle_event(&mut self, cx: &mut Cx, event: &Event, scope: &mut Scope) { - // First forward the event so dynamic StackNavigation children can produce actions. - // Then handle the Event::Actions carried by this turn. This follows the SMS/Home pattern. - self.view.handle_event(cx, event, scope); + // Forward first so PortalList children and the dynamically created + // stack view can emit their actions this turn. Same ordering the + // SMS pages use, and the reason the original scaffold commented it. + let child_actions = cx.capture_actions(|cx| { + self.view.handle_event(cx, event, scope); + }); - if let Event::Actions(actions) = event { - if self.view.button(cx, ids!(open_detail_btn)).clicked(actions) { - self.push_detail(cx); + for action in &child_actions { + // Row tapped -> open that sender's thread. + if let SharedConversationPreviewAction::Clicked { + address, + display_name, + } = action.as_widget_action().cast() + { + self.open_thread(cx, &address, &display_name); } - for action in actions { - if let StackNavigationTransitionAction::ViewReleased(view_id) = - action.as_widget_action().cast() - { - if self.current_detail_view == Some(view_id) { - self.current_detail_view = None; - } + // Back arrow / swipe dismissed the thread. + if let StackNavigationTransitionAction::ViewReleased(view_id) = + action.as_widget_action().cast() + { + if self.current_thread_view == Some(view_id) { + self.current_thread_view = None; + self.open_sender = None; } - if let StackNavigationAction::Pop = action.as_widget_action().cast() { - self.pop_detail(cx); + } + if let StackNavigationAction::Pop = action.as_widget_action().cast() { + self.close_thread(cx); + } + + // Setup form asked to connect. + if let EmailAccountSetupAction::Connect(draft) = action.as_widget_action().cast() { + self.begin_connect(cx, draft); + } + } + + // SMTP test result decides signed-in vs failed. + if let Event::Actions(actions) = event { + for action in actions { + if let Some(EmailWorkerAction::SmtpTestResult(result)) = action.downcast_ref() { + self.finish_connect(cx, result.clone()); } } } + + cx.extend_actions(child_actions); } fn draw_walk(&mut self, cx: &mut Cx2d, scope: &mut Scope, walk: Walk) -> DrawStep { - self.view.draw_walk(cx, scope, walk) + while let Some(next) = self.view.draw_walk(cx, scope, walk).step() { + // The inbox list. + if let Some(mut list) = next.as_portal_list().borrow_mut() { + let count = self.threads.len(); + // One full-height item when empty keeps the empty state + // centred and the list scrollable. + let range_end = if count == 0 { 1 } else { count + 1 }; + list.set_item_range(cx, 0, range_end); + + while let Some(item_id) = list.next_visible_item(cx) { + if count == 0 { + if item_id == 0 { + let item = list.item(cx, item_id, id!(empty_state)); + item.draw_all(cx, &mut Scope::empty()); + } + continue; + } + if item_id >= count { + let item = list.item(cx, item_id, id!(bottom_filler)); + item.draw_all(cx, &mut Scope::empty()); + continue; + } + + let thread = &self.threads[item_id]; + let item = list.item(cx, item_id, id!(conversation_preview)); + + // Populate through the shared row's documented paths. + item.label(cx, ids!(preview_content.top_row.sender_label)) + .set_text(cx, &thread.sender_display); + item.label(cx, ids!(preview_content.top_row.time_label)) + .set_text(cx, &format_thread_time(thread.latest_date_ms)); + item.label(cx, ids!(preview_content.preview_body)) + .set_text(cx, &row_body(thread)); + + // The row needs address + display name to emit Clicked. + // Bind the props to a local first: `Scope::with_props` + // borrows, so an inline temporary would be dropped + // before draw_all runs. + let props = SharedConversationPreviewProps { + address: thread.sender_address.clone(), + display_name: thread.sender_display.clone(), + was_scrolling: false, + }; + let mut item_scope = Scope::with_props(&props); + item.draw_all(cx, &mut item_scope); + } + } + } + DrawStep::done() + } +} + +/// Row body: subject then preview, which is the information order a mail +/// client uses. Subject alone is often boilerplate ("Statement ready"); +/// preview alone loses the topic. +fn row_body(thread: &EmailThreadSummary) -> String { + if thread.latest_preview.is_empty() { + thread.latest_subject.clone() + } else { + format!("{} — {}", thread.latest_subject, thread.latest_preview) + } +} + +/// Short timestamp for a list row. +fn format_thread_time(ms: i64) -> String { + use chrono::{Local, TimeZone}; + match Local.timestamp_millis_opt(ms) { + chrono::LocalResult::Single(dt) => dt.format("%d %b").to_string(), + // Provider garbage must not blank the row or panic. + _ => String::new(), } } impl EmailInboxPage { - fn push_detail(&mut self, cx: &mut Cx) { - let stack = self.view.stack_navigation(cx, ids!(page_stack)); - if stack.is_transitioning() { - return; - } - if let Some((view_id, _view)) = - stack.create_view_from_template(cx, id!(EmailInboxPageDetailStackView)) - { - self.current_detail_view = Some(view_id); - stack.set_title(cx, view_id, "Inbox"); - stack.push(cx, view_id); - cx.action(ContextNavAction::HideBottomNav); - self.view.redraw(cx); - } + /// Validate the draft and start an SMTP connection test. + fn begin_connect(&mut self, cx: &mut Cx, draft: AccountDraft) { + let (account, password) = match draft.validate() { + Ok(v) => v, + Err(errors) => { + let msg = errors + .iter() + .map(|e| e.message()) + .collect::>() + .join(" "); + self.setup_error(cx, &msg); + return; + } + }; + + self.session = SessionState::Verifying; + self.pending = Some(account.clone()); + self.password = password.clone(); + + // Reuse the existing worker. A successful SMTP handshake with + // AUTH is the only credential check available without an IMAP + // client, and it is the honest one: it proves the account can + // send, which is what this app can currently do with it. + spawn_smtp_test(SmtpConfig { + server: account.smtp_server.clone(), + port: account.smtp_port, + username: account.username.clone(), + password, + from: account.address.clone(), + }); } - fn pop_detail(&mut self, cx: &mut Cx) { + fn finish_connect(&mut self, cx: &mut Cx, result: Result<(), String>) { + let Some(account) = self.pending.take() else { + return; + }; + match result { + Ok(()) => { + self.session = SessionState::SignedIn(account); + // No receive path exists yet (assessment A1), so the list + // is populated from sample data. When a real fetch lands + // it fills `self.messages` and nothing else changes. + self.messages = sample_thread(); + self.rebuild_threads(); + self.show_signed_in(cx, true); + } + Err(reason) => { + self.setup_error(cx, &reason); + self.setup_prefill(cx, &account); + self.session = SessionState::Failed { account, reason }; + // Drop the secret the moment it is known to be unusable. + self.password.clear(); + self.show_signed_in(cx, false); + } + } + self.view.redraw(cx); + } + + /// Show a message on the setup form. + /// + /// `email_account_setup(..)` yields a `Ref`, which exposes only the + /// Widget trait; inherent methods need the concrete type, so borrow it. + fn setup_error(&mut self, cx: &mut Cx, message: &str) { + let setup = self.view.email_account_setup(cx, ids!(account_setup)); + if let Some(mut s) = setup.borrow_mut() { + s.show_error(cx, message); + } + drop(setup); + } + + /// Repopulate the form after a failure so one field can be fixed + /// without retyping the rest. + fn setup_prefill(&mut self, cx: &mut Cx, account: &EmailAccount) { + let setup = self.view.email_account_setup(cx, ids!(account_setup)); + if let Some(mut s) = setup.borrow_mut() { + s.prefill(cx, account); + } + drop(setup); + } + + fn rebuild_threads(&mut self) { + self.threads = group_by_sender(&self.messages); + } + + fn show_signed_in(&mut self, cx: &mut Cx, signed_in: bool) { + let page = if signed_in { + id!(signed_in_page) + } else { + id!(signed_out_page) + }; + self.view + .page_flip(cx, ids!(inbox_body_flip)) + .set_active_page(cx, page); + + let unread = total_unread(&self.messages); + self.view.label(cx, ids!(unread_badge)).set_text( + cx, + &if signed_in && unread > 0 { + format!("{unread} unread") + } else { + String::new() + }, + ); + self.view.redraw(cx); + } + + /// Push the thread screen for one sender. + fn open_thread(&mut self, cx: &mut Cx, sender: &str, display_name: &str) { let stack = self.view.stack_navigation(cx, ids!(page_stack)); if stack.is_transitioning() { return; } - self.current_detail_view = None; + let Some((view_id, view)) = stack.create_view_from_template(cx, id!(EmailThreadStackView)) + else { + return; + }; + + let thread = thread_for_sender(&self.messages, sender); + let subject = thread + .last() + .map(|m| m.subject.clone()) + .filter(|s| !s.trim().is_empty()) + .unwrap_or_else(|| "(no subject)".to_string()); + + view.label(cx, ids!(thread_subject)).set_text(cx, &subject); + + self.current_thread_view = Some(view_id); + self.open_sender = Some(sender.to_string()); + stack.set_title(cx, view_id, display_name); + stack.push(cx, view_id); + + // Hide the bottom nav while a thread is open, matching SMS. + cx.action(ContextNavAction::HideBottomNav); + + // Reading a thread clears its unread count. + let key = nigig_core::email_store::normalise_sender(sender); + for m in self.messages.iter_mut() { + if nigig_core::email_store::normalise_sender(&m.from_address) == key { + m.is_read = true; + } + } + self.rebuild_threads(); + let signed_in = self.session.is_signed_in(); + self.show_signed_in(cx, signed_in); + } + + fn close_thread(&mut self, cx: &mut Cx) { + let stack = self.view.stack_navigation(cx, ids!(page_stack)); + if stack.is_transitioning() { + return; + } + self.current_thread_view = None; + self.open_sender = None; stack.pop_to_root(cx); cx.action(ContextNavAction::ShowBottomNav); self.view.redraw(cx); } } + +#[cfg(test)] +mod tests { + use super::*; + + /// Row text must carry both subject and preview; a mail row showing + /// only "Statement ready" for every message is useless. + #[test] + fn row_body_combines_subject_and_preview() { + let t = EmailThreadSummary { + latest_subject: "Statement ready".into(), + latest_preview: "Your January statement is available.".into(), + ..Default::default() + }; + let body = row_body(&t); + assert!(body.contains("Statement ready")); + assert!(body.contains("January statement")); + } + + #[test] + fn row_body_falls_back_to_subject_when_there_is_no_preview() { + let t = EmailThreadSummary { + latest_subject: "(no subject)".into(), + latest_preview: String::new(), + ..Default::default() + }; + assert_eq!(row_body(&t), "(no subject)"); + } + + /// A provider timestamp we cannot represent must blank the field, not + /// panic mid-draw -- this runs inside draw_walk for every visible row. + #[test] + fn format_thread_time_never_panics_on_extreme_values() { + for ms in [i64::MIN, i64::MAX, 0, -1, 1_767_225_600_000] { + let _ = format_thread_time(ms); + } + } +} diff --git a/crates/apps/nigig-email/src/email_frame/pages/mod.rs b/crates/apps/nigig-email/src/email_frame/pages/mod.rs index 833e23f..6520513 100644 --- a/crates/apps/nigig-email/src/email_frame/pages/mod.rs +++ b/crates/apps/nigig-email/src/email_frame/pages/mod.rs @@ -1,3 +1,4 @@ +pub mod account_setup; pub mod bulk; pub mod compose; pub mod inbox; @@ -6,6 +7,7 @@ pub mod more; use makepad_widgets::ScriptVm; pub fn script_mod(vm: &mut ScriptVm) { + account_setup::script_mod(vm); inbox::script_mod(vm); compose::script_mod(vm); bulk::script_mod(vm); diff --git a/crates/apps/nigig-email/src/main.rs b/crates/apps/nigig-email/src/main.rs index aa8b2a3..d41bfce 100644 --- a/crates/apps/nigig-email/src/main.rs +++ b/crates/apps/nigig-email/src/main.rs @@ -11,6 +11,7 @@ script_mod! { window.title: "nigig-email" body +: { root := mod.widgets.StandaloneFeatureShell { + standalone_body := mod.widgets.StandaloneFeatureBody { root_screen := mod.widgets.EmailScreen {} } standalone_bottom_nav := mod.widgets.StandaloneFeatureBottomNav { diff --git a/crates/nigig-core/src/email_account.rs b/crates/nigig-core/src/email_account.rs new file mode 100644 index 0000000..fbcd995 --- /dev/null +++ b/crates/nigig-core/src/email_account.rs @@ -0,0 +1,506 @@ +//! Email account identity and session state. +//! +//! The email UI has to answer one question before it can draw anything: +//! **is an account configured?** If yes, show the inbox. If no, show the +//! setup form. Everything about that decision lives here rather than in a +//! widget, so it can be unit tested on a host with no display and no +//! network -- the same reasoning that put `BulkSendRequest::validate` and +//! `SendPacing` in `robius-sms` instead of in a Makepad page. +//! +//! ## On the password +//! +//! `SmtpConfig` in `email_worker.rs` derives `Serialize` and carries a +//! plaintext `password: String`. That is finding S2 of the assessment and +//! it is the reason this module does NOT persist the password: it stores +//! everything needed to identify and display an account, and treats the +//! secret as session-only until a keystore-backed store exists. +//! +//! Concretely: `EmailAccount` is safe to write to disk. The password is +//! held separately in memory by the caller. A future phase can add +//! platform-keystore storage (the SMS crate's `SmsScheduleCrypto.java` +//! AES-256-GCM/AndroidKeyStore path is the in-repo precedent) without +//! changing anything here. + +use serde::{Deserialize, Serialize}; + +/// A configured email account, minus the secret. +/// +/// Deliberately has no `password` field. See the module note. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct EmailAccount { + /// The user's address, e.g. `jane@example.com`. Also the SMTP `from`. + pub address: String, + /// SMTP host, e.g. `smtp.gmail.com`. + pub smtp_server: String, + pub smtp_port: u16, + /// Login name. Often the same as `address`, but not always -- some + /// providers want a bare username, so this is stored separately + /// rather than derived. + pub username: String, + /// Optional display name for outgoing mail. + pub display_name: String, +} + +/// Why an account was rejected. One variant per user-fixable mistake, so +/// the UI can say what to change instead of showing a parser error. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum AccountError { + AddressMissing, + AddressMalformed, + ServerMissing, + ServerMalformed, + PortInvalid, + UsernameMissing, + PasswordMissing, +} + +impl AccountError { + /// Message shown directly to the user. Says what to do, not what + /// failed internally. + pub fn message(&self) -> &'static str { + match self { + AccountError::AddressMissing => "Enter your email address.", + AccountError::AddressMalformed => { + "That email address does not look right. Expected something like you@example.com." + } + AccountError::ServerMissing => "Enter your provider's SMTP server.", + AccountError::ServerMalformed => { + "That server name does not look right. Expected something like smtp.example.com." + } + AccountError::PortInvalid => "Port must be a number between 1 and 65535.", + AccountError::UsernameMissing => "Enter the username for your mail account.", + AccountError::PasswordMissing => "Enter your password or app password.", + } + } +} + +/// Minimal address shape check. +/// +/// Deliberately not an RFC 5322 validator -- that grammar permits +/// quoted local parts and comments that no consumer provider accepts, and +/// a strict implementation rejects addresses that work in practice. This +/// checks the properties whose absence is always a typo: exactly one `@`, +/// non-empty both sides, a dot in the domain, no whitespace. +pub fn looks_like_email(s: &str) -> bool { + let s = s.trim(); + if s.is_empty() || s.chars().any(char::is_whitespace) { + return false; + } + let mut parts = s.split('@'); + let (Some(local), Some(domain), None) = (parts.next(), parts.next(), parts.next()) else { + return false; + }; + !local.is_empty() + && !domain.is_empty() + && domain.contains('.') + && !domain.starts_with('.') + && !domain.ends_with('.') +} + +/// Minimal hostname check: non-empty, dotted, no whitespace, no scheme. +/// +/// Rejects `https://smtp.example.com` explicitly -- pasting a URL into a +/// server field is the single most common setup mistake and produces a +/// DNS error that reads like the provider is down. +pub fn looks_like_hostname(s: &str) -> bool { + let s = s.trim(); + !s.is_empty() + && !s.contains("://") + && !s.contains('/') + && !s.chars().any(char::is_whitespace) + && s.contains('.') + && !s.starts_with('.') + && !s.ends_with('.') +} + +/// A filled-in setup form, straight from the UI. +/// +/// Strings, because that is what a `TextInput` holds. `validate` is the +/// single place that turns them into something trustworthy. +#[derive(Clone, Debug, Default)] +pub struct AccountDraft { + pub address: String, + pub smtp_server: String, + pub smtp_port: String, + pub username: String, + pub password: String, + pub display_name: String, +} + +impl AccountDraft { + /// Validate and split into (persistable account, session secret). + /// + /// Returns every error found, not just the first, so the form can + /// mark all bad fields in one pass instead of making the user + /// resubmit five times. + pub fn validate(&self) -> Result<(EmailAccount, String), Vec> { + let mut errors = Vec::new(); + + let address = self.address.trim(); + if address.is_empty() { + errors.push(AccountError::AddressMissing); + } else if !looks_like_email(address) { + errors.push(AccountError::AddressMalformed); + } + + let server = self.smtp_server.trim(); + if server.is_empty() { + errors.push(AccountError::ServerMissing); + } else if !looks_like_hostname(server) { + errors.push(AccountError::ServerMalformed); + } + + // Empty port is not an error: it means "use the default". A + // non-numeric port IS an error -- the old code did + // `parse().unwrap_or(587)`, which silently rewrote a typo and + // thereby silently changed the transport (finding B3). + let port_raw = self.smtp_port.trim(); + let port = if port_raw.is_empty() { + DEFAULT_SUBMISSION_PORT + } else { + match port_raw.parse::() { + Ok(0) | Err(_) => { + errors.push(AccountError::PortInvalid); + DEFAULT_SUBMISSION_PORT + } + Ok(p) => p, + } + }; + + // Username defaults to the address, which is right for most + // providers, so a blank field is only an error when we have no + // address to fall back on. + let username = if self.username.trim().is_empty() { + address.to_string() + } else { + self.username.trim().to_string() + }; + if username.is_empty() { + errors.push(AccountError::UsernameMissing); + } + + // Not trimmed: leading and trailing spaces can be significant in + // a password, and silently stripping them causes an auth failure + // the user cannot explain. + if self.password.is_empty() { + errors.push(AccountError::PasswordMissing); + } + + if !errors.is_empty() { + return Err(errors); + } + + Ok(( + EmailAccount { + address: address.to_string(), + smtp_server: server.to_string(), + smtp_port: port, + username, + display_name: self.display_name.trim().to_string(), + }, + self.password.clone(), + )) + } +} + +/// Default SMTP submission port (STARTTLS). +pub const DEFAULT_SUBMISSION_PORT: u16 = 587; +/// Implicit-TLS submission port. +pub const IMPLICIT_TLS_PORT: u16 = 465; + +/// Guess SMTP settings from an address, so the common case is one field. +/// +/// Returns `None` for domains we do not know, in which case the user must +/// supply the server themselves. Guessing `smtp.` for arbitrary +/// domains is tempting and wrong: it is right often enough to look like a +/// feature and wrong often enough to produce confusing failures. +pub fn guess_provider(address: &str) -> Option<(&'static str, u16)> { + let domain = address.trim().rsplit_once('@')?.1.to_ascii_lowercase(); + let settings = match domain.as_str() { + "gmail.com" | "googlemail.com" => ("smtp.gmail.com", IMPLICIT_TLS_PORT), + "outlook.com" | "hotmail.com" | "live.com" | "msn.com" => { + ("smtp-mail.outlook.com", DEFAULT_SUBMISSION_PORT) + } + "yahoo.com" | "ymail.com" => ("smtp.mail.yahoo.com", IMPLICIT_TLS_PORT), + "icloud.com" | "me.com" | "mac.com" => ("smtp.mail.me.com", DEFAULT_SUBMISSION_PORT), + "zoho.com" => ("smtp.zoho.com", IMPLICIT_TLS_PORT), + "protonmail.com" | "proton.me" => ("smtp.protonmail.ch", DEFAULT_SUBMISSION_PORT), + _ => return None, + }; + Some(settings) +} + +/// What the email screen should show. +/// +/// The UI reads exactly this to decide between the inbox and the setup +/// form. Keeping it an enum rather than a bare `bool` leaves room for +/// `Verifying` without another refactor. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub enum SessionState { + /// No account configured: show the setup form. + #[default] + SignedOut, + /// Credentials submitted, connection test in flight. + Verifying, + /// Account usable: show the inbox. + SignedIn(EmailAccount), + /// Verification failed. Carries the message to display, and keeps the + /// draft-derived account so the form can be repopulated rather than + /// making the user retype everything. + Failed { + account: EmailAccount, + reason: String, + }, +} + +impl SessionState { + /// Should the inbox be shown? + pub fn is_signed_in(&self) -> bool { + matches!(self, SessionState::SignedIn(_)) + } + + /// The account, if there is one to display -- including after a + /// failure, so the form can be prefilled. + pub fn account(&self) -> Option<&EmailAccount> { + match self { + SessionState::SignedIn(a) | SessionState::Failed { account: a, .. } => Some(a), + _ => None, + } + } + + /// Text for the account row in the More page. + pub fn status_line(&self) -> String { + match self { + SessionState::SignedOut => "No account connected".into(), + SessionState::Verifying => "Checking connection…".into(), + SessionState::SignedIn(a) => format!("Connected as {}", a.address), + SessionState::Failed { account, reason } => { + format!("{} — not connected: {}", account.address, reason) + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn good_draft() -> AccountDraft { + AccountDraft { + address: "jane@example.com".into(), + smtp_server: "smtp.example.com".into(), + smtp_port: "587".into(), + username: "jane@example.com".into(), + password: "hunter2".into(), + display_name: "Jane".into(), + } + } + + #[test] + fn a_complete_draft_validates() { + let (account, secret) = good_draft().validate().expect("should validate"); + assert_eq!(account.address, "jane@example.com"); + assert_eq!(account.smtp_port, 587); + assert_eq!(secret, "hunter2"); + } + + /// The persistable half must never carry the secret. This is the + /// structural half of finding S2: if EmailAccount grows a password + /// field, this test is where it should be noticed. + #[test] + fn the_account_type_has_no_password_field() { + let (account, _) = good_draft().validate().unwrap(); + let json = serde_json::to_string(&account).unwrap(); + assert!( + !json.contains("hunter2"), + "serialised account leaked the password: {json}" + ); + assert!( + !json.contains("password"), + "account has a password field: {json}" + ); + } + + #[test] + fn every_error_is_reported_not_just_the_first() { + let draft = AccountDraft::default(); + let errors = draft.validate().unwrap_err(); + assert!(errors.contains(&AccountError::AddressMissing)); + assert!(errors.contains(&AccountError::ServerMissing)); + assert!(errors.contains(&AccountError::PasswordMissing)); + assert!( + errors.len() >= 3, + "expected several errors in one pass, got {errors:?}" + ); + } + + /// B3: the old code did `parse().unwrap_or(587)`, so "465x" silently + /// became 587 and silently changed the transport. It must be an error. + #[test] + fn a_malformed_port_is_an_error_not_a_silent_default() { + for bad in ["465x", "abc", "-1", "99999", "0", "5 8 7"] { + let draft = AccountDraft { + smtp_port: bad.into(), + ..good_draft() + }; + let errors = draft.validate().unwrap_err(); + assert!( + errors.contains(&AccountError::PortInvalid), + "port {bad:?} should be rejected, got {errors:?}" + ); + } + } + + #[test] + fn an_empty_port_means_the_default() { + let draft = AccountDraft { + smtp_port: " ".into(), + ..good_draft() + }; + let (account, _) = draft.validate().unwrap(); + assert_eq!(account.smtp_port, DEFAULT_SUBMISSION_PORT); + } + + #[test] + fn username_falls_back_to_the_address() { + let draft = AccountDraft { + username: "".into(), + ..good_draft() + }; + let (account, _) = draft.validate().unwrap(); + assert_eq!(account.username, "jane@example.com"); + } + + /// Spaces can be significant in a password; trimming causes an auth + /// failure the user cannot diagnose. + #[test] + fn the_password_is_not_trimmed() { + let draft = AccountDraft { + password: " spaced ".into(), + ..good_draft() + }; + let (_, secret) = draft.validate().unwrap(); + assert_eq!(secret, " spaced "); + } + + #[test] + fn accepts_ordinary_addresses() { + for good in [ + "a@b.co", + "jane.doe@example.com", + "jane+tag@example.co.ke", + "j_d-1@sub.example.org", + ] { + assert!(looks_like_email(good), "should accept {good}"); + } + } + + #[test] + fn rejects_addresses_that_are_always_typos() { + for bad in [ + "", + " ", + "no-at-sign", + "@example.com", + "jane@", + "jane@nodot", + "two@at@signs.com", + "jane doe@example.com", + "jane@.example.com", + "jane@example.", + ] { + assert!(!looks_like_email(bad), "should reject {bad:?}"); + } + } + + /// Pasting a URL into the server field is the most common setup + /// mistake and yields a DNS error that looks like an outage. + #[test] + fn a_url_in_the_server_field_is_rejected() { + for bad in [ + "https://smtp.example.com", + "smtp://smtp.example.com", + "smtp.example.com/path", + ] { + assert!(!looks_like_hostname(bad), "should reject {bad:?}"); + let draft = AccountDraft { + smtp_server: bad.into(), + ..good_draft() + }; + let errors = draft.validate().unwrap_err(); + assert!(errors.contains(&AccountError::ServerMalformed)); + } + } + + #[test] + fn guesses_settings_for_the_common_providers() { + assert_eq!( + guess_provider("jane@gmail.com"), + Some(("smtp.gmail.com", IMPLICIT_TLS_PORT)) + ); + assert_eq!( + guess_provider("JANE@GMail.COM"), + Some(("smtp.gmail.com", IMPLICIT_TLS_PORT)), + "domain match must be case-insensitive" + ); + assert!(guess_provider("jane@outlook.com").is_some()); + assert!(guess_provider("jane@yahoo.com").is_some()); + } + + /// Guessing smtp. for unknown hosts is wrong often enough to + /// be worse than asking. + #[test] + fn does_not_guess_for_unknown_domains() { + assert_eq!(guess_provider("jane@my-company.co.ke"), None); + assert_eq!(guess_provider("not-an-address"), None); + assert_eq!(guess_provider(""), None); + } + + #[test] + fn session_state_gates_the_inbox() { + let (account, _) = good_draft().validate().unwrap(); + assert!(!SessionState::SignedOut.is_signed_in()); + assert!(!SessionState::Verifying.is_signed_in()); + assert!(SessionState::SignedIn(account.clone()).is_signed_in()); + assert!( + !SessionState::Failed { + account: account.clone(), + reason: "bad password".into() + } + .is_signed_in(), + "a failed session must NOT show the inbox" + ); + } + + /// After a failure the form must be repopulatable, or the user + /// retypes five fields to fix one. + #[test] + fn a_failed_session_still_exposes_the_account_for_the_form() { + let (account, _) = good_draft().validate().unwrap(); + let failed = SessionState::Failed { + account: account.clone(), + reason: "auth failed".into(), + }; + assert_eq!(failed.account(), Some(&account)); + } + + #[test] + fn status_lines_are_never_empty_and_name_the_account() { + let (account, _) = good_draft().validate().unwrap(); + for state in [ + SessionState::SignedOut, + SessionState::Verifying, + SessionState::SignedIn(account.clone()), + SessionState::Failed { + account, + reason: "nope".into(), + }, + ] { + assert!(!state.status_line().is_empty()); + } + let (a, _) = good_draft().validate().unwrap(); + assert!(SessionState::SignedIn(a) + .status_line() + .contains("jane@example.com")); + } +} diff --git a/crates/nigig-core/src/email_store.rs b/crates/nigig-core/src/email_store.rs new file mode 100644 index 0000000..d0545ec --- /dev/null +++ b/crates/nigig-core/src/email_store.rs @@ -0,0 +1,526 @@ +//! Email message model and sender-thread grouping. +//! +//! The SMS inbox groups messages by `address` into conversations, and +//! tapping one opens a timeline. Email needs the same shape: a list of +//! senders, and a thread per sender. This module is that grouping, as +//! pure functions over plain data. +//! +//! Deliberately transport-free. There is no IMAP client in this +//! repository yet (assessment finding A1), and the receive strategy -- +//! IMAP on device vs a server-side proxy -- is still an open product +//! decision. Rather than block the UI on it, this defines the model the +//! UI binds to and provides an explicit `sample_thread` for development. +//! When a real fetch lands it populates these same types and the UI does +//! not change. +//! +//! Host-testable on purpose: grouping, sorting and preview truncation are +//! where the bugs live, and none of them need a network. + +use serde::{Deserialize, Serialize}; + +/// One message. `Serialize` is for a future local cache; note that unlike +/// `SmsConfig` there is no secret in here, so it is safe. +/// +/// The body is NOT `#[serde(skip)]`-ed the way `OfflineSmsMessage.body` +/// is, because email bodies are the thing the user came to read and a +/// cache that drops them is useless. When persistence lands it must be +/// encrypted at rest instead -- tracked as Phase C3. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct EmailMessage { + /// Provider-assigned id, or a synthesised one for local drafts. + pub id: String, + /// Envelope sender address, e.g. `alerts@bank.co.ke`. + pub from_address: String, + /// Human name if the provider supplied one, else empty. + pub from_name: String, + pub subject: String, + pub body: String, + /// Epoch millis. i64 so pre-1970 and provider garbage cannot panic. + pub date_ms: i64, + pub is_read: bool, + /// True when this message was sent by the account owner, which + /// decides which side of the timeline it renders on. + pub is_outgoing: bool, +} + +impl EmailMessage { + /// Best available display name for the sender. + pub fn sender_display(&self) -> &str { + if self.from_name.trim().is_empty() { + &self.from_address + } else { + &self.from_name + } + } +} + +/// One row in the inbox list: a sender and the state of their thread. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct EmailThreadSummary { + /// Grouping key -- the normalised sender address. + pub sender_address: String, + pub sender_display: String, + /// Subject of the most recent message, used as the row title. + pub latest_subject: String, + /// Body preview of the most recent message. + pub latest_preview: String, + pub latest_date_ms: i64, + pub unread_count: usize, + pub message_count: usize, +} + +/// Normalise an address for grouping. +/// +/// Lowercased and trimmed, because `Alerts@Bank.co.ke` and +/// `alerts@bank.co.ke` are one sender and showing them as two rows is the +/// email equivalent of the SMS duplicate-recipient bug. +pub fn normalise_sender(address: &str) -> String { + address.trim().to_ascii_lowercase() +} + +/// Truncate a body to a single-line preview. +/// +/// **Char-safe.** `&s[..n]` panics when `n` is not a UTF-8 boundary, which +/// is bug A3 in the SMS crate: one inbound message containing emoji or +/// non-Latin text took down the whole list on every frame. Email bodies +/// are equally untrusted and equally likely to contain both, and there is +/// a CI gate in this repo forbidding byte-offset slicing in SMS text +/// helpers for exactly this reason. Uses `char_indices`. +pub fn preview_line(body: &str, max_chars: usize) -> String { + // Collapse all whitespace first: a body starting with ten newlines + // otherwise previews as an empty row. + let flat: String = body.split_whitespace().collect::>().join(" "); + if max_chars == 0 { + return String::new(); + } + let mut count = 0; + for (idx, _) in flat.char_indices() { + if count == max_chars { + // idx is a real char boundary from char_indices. + return format!("{}…", &flat[..idx]); + } + count += 1; + } + flat +} + +/// Default preview length, matching the SMS list's density. +pub const PREVIEW_CHARS: usize = 78; + +/// Group messages into one summary per sender, newest thread first. +/// +/// Outgoing messages are included in a thread's count and can supply its +/// preview -- a thread you replied to should show your reply as the latest +/// activity, the way SMS conversations do -- but they never count as +/// unread. +pub fn group_by_sender(messages: &[EmailMessage]) -> Vec { + use std::collections::HashMap; + + let mut by_sender: HashMap = HashMap::new(); + + for msg in messages { + let key = normalise_sender(&msg.from_address); + if key.is_empty() { + continue; + } + let entry = by_sender + .entry(key.clone()) + .or_insert_with(|| EmailThreadSummary { + sender_address: key.clone(), + sender_display: msg.sender_display().to_string(), + latest_date_ms: i64::MIN, + ..Default::default() + }); + + entry.message_count += 1; + if !msg.is_read && !msg.is_outgoing { + entry.unread_count += 1; + } + + // Only the newest message supplies the row's title and preview. + if msg.date_ms >= entry.latest_date_ms { + entry.latest_date_ms = msg.date_ms; + entry.latest_subject = if msg.subject.trim().is_empty() { + "(no subject)".to_string() + } else { + msg.subject.clone() + }; + entry.latest_preview = preview_line(&msg.body, PREVIEW_CHARS); + // Prefer a real name over a bare address if any message in + // the thread has one. + if !msg.from_name.trim().is_empty() { + entry.sender_display = msg.from_name.clone(); + } + } + } + + let mut out: Vec = by_sender.into_values().collect(); + // Newest first; tie-break on address so the order is deterministic + // rather than HashMap-iteration order, which would make the list + // reshuffle between frames. + out.sort_by(|a, b| { + b.latest_date_ms + .cmp(&a.latest_date_ms) + .then_with(|| a.sender_address.cmp(&b.sender_address)) + }); + out +} + +/// All messages from one sender, oldest first (timeline order). +pub fn thread_for_sender(messages: &[EmailMessage], sender_address: &str) -> Vec { + let key = normalise_sender(sender_address); + let mut out: Vec = messages + .iter() + .filter(|m| normalise_sender(&m.from_address) == key) + .cloned() + .collect(); + out.sort_by(|a, b| a.date_ms.cmp(&b.date_ms).then_with(|| a.id.cmp(&b.id))); + out +} + +/// Total unread across all threads, for the tab badge. +pub fn total_unread(messages: &[EmailMessage]) -> usize { + messages + .iter() + .filter(|m| !m.is_read && !m.is_outgoing) + .count() +} + +/// Case-insensitive substring filter over sender, subject and body. +pub fn filter_messages(messages: &[EmailMessage], needle: &str) -> Vec { + let n = needle.trim().to_ascii_lowercase(); + if n.is_empty() { + return messages.to_vec(); + } + messages + .iter() + .filter(|m| { + m.from_address.to_ascii_lowercase().contains(&n) + || m.from_name.to_ascii_lowercase().contains(&n) + || m.subject.to_ascii_lowercase().contains(&n) + || m.body.to_ascii_lowercase().contains(&n) + }) + .cloned() + .collect() +} + +/// Development sample data. +/// +/// There is no receive path yet, so without this the inbox can only ever +/// render an empty state and the list/thread transition cannot be +/// exercised at all. Explicitly named `sample_` so it is obvious in a +/// diff when the real fetch replaces it. +pub fn sample_thread() -> Vec { + let base = 1_767_225_600_000i64; // 2026-01-01T00:00:00Z + let hour = 3_600_000i64; + vec![ + EmailMessage { + id: "s1".into(), + from_address: "alerts@bank.co.ke".into(), + from_name: "Equity Alerts".into(), + subject: "Statement ready".into(), + body: "Your January statement is now available in the portal.".into(), + date_ms: base, + is_read: false, + is_outgoing: false, + }, + EmailMessage { + id: "s2".into(), + from_address: "alerts@bank.co.ke".into(), + from_name: "Equity Alerts".into(), + subject: "Statement ready".into(), + body: "Reminder: your January statement is still unread.".into(), + date_ms: base + hour, + is_read: false, + is_outgoing: false, + }, + EmailMessage { + id: "s3".into(), + from_address: "jane@example.com".into(), + from_name: "Jane Wanjiku".into(), + subject: "Re: delivery schedule".into(), + body: "Wednesday works for the Nairobi drop-off. Confirming now.".into(), + date_ms: base + 4 * hour, + is_read: true, + is_outgoing: false, + }, + EmailMessage { + id: "s4".into(), + from_address: "jane@example.com".into(), + from_name: "Jane Wanjiku".into(), + subject: "Re: delivery schedule".into(), + body: "Thanks Jane, Wednesday is confirmed on our side too.".into(), + date_ms: base + 5 * hour, + is_read: true, + is_outgoing: true, + }, + EmailMessage { + id: "s5".into(), + from_address: "no-reply@marikiti.co.ke".into(), + from_name: String::new(), + subject: String::new(), + body: "Order #4471 dispatched. Track it in the app. 🚚".into(), + date_ms: base + 8 * hour, + is_read: false, + is_outgoing: false, + }, + ] +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn groups_messages_into_one_row_per_sender() { + let threads = group_by_sender(&sample_thread()); + assert_eq!(threads.len(), 3, "3 distinct senders in the sample"); + } + + #[test] + fn newest_thread_comes_first() { + let threads = group_by_sender(&sample_thread()); + assert_eq!(threads[0].sender_address, "no-reply@marikiti.co.ke"); + assert!(threads[0].latest_date_ms >= threads[1].latest_date_ms); + assert!(threads[1].latest_date_ms >= threads[2].latest_date_ms); + } + + /// Address case must not split one sender into two rows. + #[test] + fn sender_grouping_is_case_insensitive() { + let msgs = vec![ + EmailMessage { + id: "a".into(), + from_address: "Alerts@Bank.CO.KE".into(), + date_ms: 1, + ..Default::default() + }, + EmailMessage { + id: "b".into(), + from_address: "alerts@bank.co.ke".into(), + date_ms: 2, + ..Default::default() + }, + ]; + let threads = group_by_sender(&msgs); + assert_eq!(threads.len(), 1, "same sender, different case"); + assert_eq!(threads[0].message_count, 2); + } + + #[test] + fn unread_counts_exclude_our_own_sent_mail() { + let msgs = vec![ + EmailMessage { + id: "in".into(), + from_address: "a@b.com".into(), + is_read: false, + is_outgoing: false, + date_ms: 1, + ..Default::default() + }, + EmailMessage { + id: "out".into(), + from_address: "a@b.com".into(), + is_read: false, + is_outgoing: true, + date_ms: 2, + ..Default::default() + }, + ]; + let threads = group_by_sender(&msgs); + assert_eq!( + threads[0].unread_count, 1, + "outgoing must not count as unread" + ); + assert_eq!(threads[0].message_count, 2); + assert_eq!(total_unread(&msgs), 1); + } + + /// A reply should surface as the thread's latest activity. + #[test] + fn an_outgoing_reply_can_supply_the_preview() { + let threads = group_by_sender(&sample_thread()); + let jane = threads + .iter() + .find(|t| t.sender_address == "jane@example.com") + .unwrap(); + assert!( + jane.latest_preview.contains("confirmed on our side"), + "expected the reply as latest, got {:?}", + jane.latest_preview + ); + } + + #[test] + fn an_empty_subject_reads_as_no_subject_not_blank() { + let threads = group_by_sender(&sample_thread()); + let m = threads + .iter() + .find(|t| t.sender_address == "no-reply@marikiti.co.ke") + .unwrap(); + assert_eq!(m.latest_subject, "(no subject)"); + } + + #[test] + fn sender_display_falls_back_to_the_address() { + let m = EmailMessage { + from_address: "x@y.com".into(), + from_name: " ".into(), + ..Default::default() + }; + assert_eq!(m.sender_display(), "x@y.com"); + } + + #[test] + fn a_blank_sender_is_dropped_rather_than_grouped_under_empty() { + let msgs = vec![EmailMessage { + from_address: " ".into(), + date_ms: 1, + ..Default::default() + }]; + assert!(group_by_sender(&msgs).is_empty()); + } + + #[test] + fn thread_is_oldest_first_for_timeline_order() { + let t = thread_for_sender(&sample_thread(), "alerts@bank.co.ke"); + assert_eq!(t.len(), 2); + assert!(t[0].date_ms < t[1].date_ms); + } + + #[test] + fn thread_lookup_is_case_insensitive() { + let t = thread_for_sender(&sample_thread(), "ALERTS@BANK.CO.KE"); + assert_eq!(t.len(), 2); + } + + #[test] + fn thread_for_an_unknown_sender_is_empty_not_a_panic() { + assert!(thread_for_sender(&sample_thread(), "nobody@nowhere.test").is_empty()); + } + + // ---- preview_line: the A3 class of bug ------------------------------- + + /// The whole reason this is a function and not `&body[..78]`. + #[test] + fn preview_never_panics_on_multibyte_text() { + for body in [ + "🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚🚚", + "Habari ya asubuhi, tunatumaini kwamba unaendelea vizuri sana leo hii rafiki", + "مرحبا بك في تطبيقنا الجديد نرجو أن تستمتع بالتجربة", + "こんにちは、これはとても長いメッセージのテストです。よろしくお願いします。", + "Hi 🚚 mixed ascii and emoji that lands mid-character at the boundary", + ] { + let out = preview_line(body, PREVIEW_CHARS); + assert!(!out.is_empty(), "empty preview for {body:?}"); + } + } + + /// Misaligned multibyte text is the case that actually triggered A3 -- + /// uniform emoji happens to land on a boundary. + #[test] + fn preview_truncates_misaligned_multibyte_at_a_char_boundary() { + let body = format!("Hi {}", "🚚".repeat(200)); + let out = preview_line(&body, 40); + assert!(out.ends_with('…')); + assert_eq!(out.chars().count(), 41, "40 chars plus the ellipsis"); + } + + #[test] + fn preview_collapses_whitespace_so_a_row_is_never_blank() { + let out = preview_line("\n\n\n \t hello world \n", 40); + assert_eq!(out, "hello world"); + } + + #[test] + fn short_bodies_are_returned_whole_without_an_ellipsis() { + let out = preview_line("short", PREVIEW_CHARS); + assert_eq!(out, "short"); + assert!(!out.ends_with('…')); + } + + #[test] + fn preview_of_zero_chars_is_empty_not_a_panic() { + assert_eq!(preview_line("anything", 0), ""); + } + + #[test] + fn preview_at_exactly_the_limit_is_not_truncated() { + let body = "a".repeat(10); + assert_eq!(preview_line(&body, 10), body); + } + + // ---- filtering ------------------------------------------------------- + + #[test] + fn filter_matches_sender_subject_and_body_case_insensitively() { + let all = sample_thread(); + assert_eq!(filter_messages(&all, "EQUITY").len(), 2, "by sender name"); + assert_eq!(filter_messages(&all, "delivery").len(), 2, "by subject"); + assert_eq!(filter_messages(&all, "dispatched").len(), 1, "by body"); + assert_eq!(filter_messages(&all, "bank.co.ke").len(), 2, "by address"); + } + + #[test] + fn an_empty_filter_returns_everything() { + let all = sample_thread(); + assert_eq!(filter_messages(&all, " ").len(), all.len()); + } + + #[test] + fn a_filter_matching_nothing_returns_empty_not_everything() { + assert!(filter_messages(&sample_thread(), "zzzz-no-match").is_empty()); + } + + /// HashMap iteration order must not leak into the UI, or rows + /// reshuffle between frames. + #[test] + fn grouping_is_deterministic_across_runs() { + let msgs = sample_thread(); + let a = group_by_sender(&msgs); + for _ in 0..20 { + assert_eq!(group_by_sender(&msgs), a); + } + } + + #[test] + fn ties_on_timestamp_are_broken_deterministically() { + let msgs = vec![ + EmailMessage { + id: "1".into(), + from_address: "b@x.com".into(), + date_ms: 5, + ..Default::default() + }, + EmailMessage { + id: "2".into(), + from_address: "a@x.com".into(), + date_ms: 5, + ..Default::default() + }, + ]; + let t = group_by_sender(&msgs); + assert_eq!(t[0].sender_address, "a@x.com", "tie broken by address"); + } + + #[test] + fn extreme_timestamps_do_not_panic() { + let msgs = vec![ + EmailMessage { + id: "min".into(), + from_address: "a@x.com".into(), + date_ms: i64::MIN, + ..Default::default() + }, + EmailMessage { + id: "max".into(), + from_address: "b@x.com".into(), + date_ms: i64::MAX, + ..Default::default() + }, + ]; + let t = group_by_sender(&msgs); + assert_eq!(t.len(), 2); + assert_eq!(t[0].sender_address, "b@x.com"); + } +} diff --git a/crates/nigig-core/src/lib.rs b/crates/nigig-core/src/lib.rs index d4bfec8..dc4cb6f 100644 --- a/crates/nigig-core/src/lib.rs +++ b/crates/nigig-core/src/lib.rs @@ -18,6 +18,8 @@ pub mod platform; pub mod syncing; #[cfg(not(target_arch = "wasm32"))] pub mod tile_service; +pub mod email_account; +pub mod email_store; pub mod email_worker; pub use dir::app_data_dir;