refactor(messages): MessageActor enum for render-time actor identity (LC-77) #194

Merged

longjacksonle merged 2 commits from feat/lc-77-message-actor-enum into main

2026-05-25 18:21:51 +02:00

longjacksonle commented

2026-05-25 18:19:04 +02:00

Owner

Summary

Replace MessageView::author_is_webhook + webhook_avatar_url (a bool + Option pair) with a single actor: MessageActor sum type. This is the LC-62 "refactor before feature" prep for LC-77 per-room email ingress: the email synthetic actor lands as MessageActor::EmailInbox in the next commit, sitting cleanly next to the existing Webhook and User arms.

Behavior-preserving for LC-74 incoming webhooks. The fixture committed in the first commit on this branch pins the rendered HTML byte-for-byte; the refactor commit's identical rendered output is the proof.

Why this refactor before the email feature

Reply-by-email (the deferred LC-77 sibling) is a known fourth case. Extract-the-abstraction-now is the right call when the next caller is committed (the email arm IS committed, scheduled for the very next commit), not speculative. Doing the boolean -> enum conversion mid-feature would either bloat the email PR or leave LC-74 with a stopgap shape that would need unwinding later.

Behavior preservation

tests/lc77_webhook_render_fixture.rs constructs MessageView with deterministic values for two cases (webhook without avatar URL, webhook with avatar URL), renders via NewMessageFragment (the same fragment the WS hub broadcasts), and asserts the rendered HTML matches the committed fixture files under tests/fixtures/.

Fixture files captured in the first commit (BEFORE the refactor). The refactor commit's cargo test passes byte-equal on both fixtures, no DOM-equal fallback needed. The Askama {% match %} emitted identical whitespace to the previous {% if %} chain.

What changed

New module: server/src/views/message_actor.rs. MessageActor enum with User unit variant and Webhook(Option<String>) tuple variant carrying the webhook avatar URL. Tuple variant chosen to match the only Askama match precedent in this repo (room/moderators.html uses {% when Some with (d) %}). Constructor MessageActor::from_webhook_flag(is_webhook, avatar_url) keeps the if/else logic in one place so every construction site stays one line.
MessageView: author_is_webhook: bool and webhook_avatar_url: Option<String> removed; replaced with actor: MessageActor.
templates/room/message.html lines 17-42: {% if message.author_is_webhook %} converted to {% match message.actor %} with explicit Webhook and User arms. The template uses the fully qualified path crate::views::message_actor::MessageActor::... so no import is needed in the Template-deriving modules that include this partial (views/ws_fragments.rs, views/room.rs, views/dm.rs).
11 construction sites updated: routes/mod.rs (1), routes/dm.rs (1), routes/room.rs (4), routes/ws.rs (3), tests/lc77_webhook_render_fixture.rs (1, post-refactor shape). Each calls MessageActor::from_webhook_flag(...).

Anti-scope

No EmailInbox arm. That arrives with the schema in the next commit.
No user-identity scalar move into the User variant. Real-user identity fields (user_id, username, avatar_ext, status, custom_status, author_is_bot) stay on MessageView since they're shared across actor types. The User variant is intentionally unit.
No accessor methods on MessageActor. The template branches via {% match %} directly. If commit 3 needs accessors for the EmailInbox arm they can be added then.
No link-filter posture change. Webhook gates remain unchanged. The named link-filter decision for email ingress lands in commit 5 of LC-77.

Test plan

cargo check --workspace clean.
cargo check --no-default-features --features lets-chat-server/saas clean.
cargo test -p lets-chat-server --tests all 50 binaries pass under default features.
cargo test -p lets-chat-server --tests --no-default-features --features saas all 50 binaries pass under saas.
cargo clippy -p lets-chat-server -- -D warnings clean.
cargo fmt --check clean for files in this PR. Pre-existing fmt diffs in views/math.rs + views/markdown.rs predate this branch (LC-59 carryover on main); separate hygiene followup ticket tracked.
Fixture tests pass byte-equal in verify mode (no DOM-equal fallback engaged).

Followups out of scope for this PR

Tracked as LC-77 sub-issues so they don't evaporate: LC-77-SMTP-SEAL (migrate SMTP password to VAPID-sealed pattern), LC-77-REPLY (reply-by-email), LC-77-MID-DEDUP (exactly-once dedup), LC-77-DEAD-LETTER (optional dead-letter IMAP folder), LC-77-FMT-CARRYOVER (math.rs/markdown.rs fmt cleanup from LC-59).

## Summary Replace `MessageView::author_is_webhook` + `webhook_avatar_url` (a bool + Option pair) with a single `actor: MessageActor` sum type. This is the LC-62 "refactor before feature" prep for LC-77 per-room email ingress: the email synthetic actor lands as `MessageActor::EmailInbox` in the next commit, sitting cleanly next to the existing `Webhook` and `User` arms. Behavior-preserving for LC-74 incoming webhooks. The fixture committed in the first commit on this branch pins the rendered HTML byte-for-byte; the refactor commit's identical rendered output is the proof. ## Why this refactor before the email feature Reply-by-email (the deferred LC-77 sibling) is a known fourth case. Extract-the-abstraction-now is the right call when the next caller is committed (the email arm IS committed, scheduled for the very next commit), not speculative. Doing the boolean -> enum conversion mid-feature would either bloat the email PR or leave LC-74 with a stopgap shape that would need unwinding later. ## Behavior preservation `tests/lc77_webhook_render_fixture.rs` constructs `MessageView` with deterministic values for two cases (webhook without avatar URL, webhook with avatar URL), renders via `NewMessageFragment` (the same fragment the WS hub broadcasts), and asserts the rendered HTML matches the committed fixture files under `tests/fixtures/`. Fixture files captured in the first commit (BEFORE the refactor). The refactor commit's `cargo test` passes byte-equal on both fixtures, no DOM-equal fallback needed. The Askama `{% match %}` emitted identical whitespace to the previous `{% if %}` chain. ## What changed - **New module**: `server/src/views/message_actor.rs`. `MessageActor` enum with `User` unit variant and `Webhook(Option<String>)` tuple variant carrying the webhook avatar URL. Tuple variant chosen to match the only Askama match precedent in this repo (`room/moderators.html` uses `{% when Some with (d) %}`). Constructor `MessageActor::from_webhook_flag(is_webhook, avatar_url)` keeps the if/else logic in one place so every construction site stays one line. - **`MessageView`**: `author_is_webhook: bool` and `webhook_avatar_url: Option<String>` removed; replaced with `actor: MessageActor`. - **`templates/room/message.html` lines 17-42**: `{% if message.author_is_webhook %}` converted to `{% match message.actor %}` with explicit `Webhook` and `User` arms. The template uses the fully qualified path `crate::views::message_actor::MessageActor::...` so no import is needed in the Template-deriving modules that include this partial (`views/ws_fragments.rs`, `views/room.rs`, `views/dm.rs`). - **11 construction sites updated**: `routes/mod.rs` (1), `routes/dm.rs` (1), `routes/room.rs` (4), `routes/ws.rs` (3), `tests/lc77_webhook_render_fixture.rs` (1, post-refactor shape). Each calls `MessageActor::from_webhook_flag(...)`. ## Anti-scope - No EmailInbox arm. That arrives with the schema in the next commit. - No user-identity scalar move into the User variant. Real-user identity fields (`user_id`, `username`, `avatar_ext`, `status`, `custom_status`, `author_is_bot`) stay on `MessageView` since they're shared across actor types. The User variant is intentionally unit. - No accessor methods on `MessageActor`. The template branches via `{% match %}` directly. If commit 3 needs accessors for the EmailInbox arm they can be added then. - No link-filter posture change. Webhook gates remain unchanged. The named link-filter decision for email ingress lands in commit 5 of LC-77. ## Test plan - [x] `cargo check --workspace` clean. - [x] `cargo check --no-default-features --features lets-chat-server/saas` clean. - [x] `cargo test -p lets-chat-server --tests` all 50 binaries pass under default features. - [x] `cargo test -p lets-chat-server --tests --no-default-features --features saas` all 50 binaries pass under saas. - [x] `cargo clippy -p lets-chat-server -- -D warnings` clean. - [x] `cargo fmt --check` clean for files in this PR. Pre-existing fmt diffs in `views/math.rs` + `views/markdown.rs` predate this branch (LC-59 carryover on `main`); separate hygiene followup ticket tracked. - [x] Fixture tests pass byte-equal in verify mode (no DOM-equal fallback engaged). ## Followups out of scope for this PR Tracked as LC-77 sub-issues so they don't evaporate: `LC-77-SMTP-SEAL` (migrate SMTP password to VAPID-sealed pattern), `LC-77-REPLY` (reply-by-email), `LC-77-MID-DEDUP` (exactly-once dedup), `LC-77-DEAD-LETTER` (optional dead-letter IMAP folder), `LC-77-FMT-CARRYOVER` (math.rs/markdown.rs fmt cleanup from LC-59).

longjacksonle added 2 commits

2026-05-25 18:19:04 +02:00

test(messages): pin webhook render output as pre-refactor fixture (LC-77) 0182bbc443

Baseline for the LC-77 MessageActor enum refactor that lands in the next commit. The refactor replaces the author_is_webhook + webhook_avatar_url fields on MessageView with a sum type. This commit pins the current rendered HTML so the refactor must reproduce it byte-equal (or DOM-equal in fallback).

Two cases:
- Webhook without an avatar URL: renders initials fallback (room/message.html line 22).
- Webhook with an avatar URL: renders an img tag (room/message.html line 20).

Tests construct MessageView directly via the public fields with deterministic values, render via NewMessageFragment (the same fragment the WS hub broadcasts for new messages), and compare to the committed fixture files under tests/fixtures/.

Regenerate fixtures with FIXTURE_WRITE=1 in the test process environment if the rendered HTML changes intentionally; the env-var gate keeps the verify path strict by default.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

refactor(messages): MessageActor enum for render-time actor identity (LC-77)

check-secrets / Nosey parker (push) Successful in 3s

Details

check-secrets / TruffleHog (push) Successful in 3s

Details

check-secrets / Kingfisher (push) Successful in 5s

Details

check-secrets / Nosey parker (pull_request) Successful in 3s

Details

check-secrets / Kingfisher (pull_request) Successful in 4s

Details

check-secrets / TruffleHog (pull_request) Successful in 5s

Details

Check / clippy + fmt + tests (pull_request) Failing after 14s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

1d9b210104

Replace MessageView's author_is_webhook + webhook_avatar_url scalar pair with a single actor: MessageActor field. Behavior-preserving for LC-74 webhooks: the byte-equal fixture pinned in commit 0182bbc passes on both webhook render cases (with and without avatar URL) without needing the DOM-equal fallback. The {% match %} Askama syntax emitted identical whitespace to the previous {% if %} chain.

This refactor sets up the data model for the LC-77 email-ingress synthetic actor (the EmailInbox variant lands in the commit that introduces the email_inboxes schema). Reply-by-email, the deferred LC-77 sibling, becomes the fourth case when it ships.

Changes:
- views/message_actor.rs (new): MessageActor enum. User unit variant; Webhook(Option<String>) tuple variant carrying the webhook avatar URL. Tuple chosen to match the only Askama match precedent in this repo (room/moderators.html uses {% when Some with (d) %}). Constructor from_webhook_flag(is_webhook, avatar_url) keeps the if/else logic in one place.
- views/room.rs: MessageView::author_is_webhook and webhook_avatar_url removed; replaced with actor: MessageActor.
- templates/room/message.html lines 17-42: {% if message.author_is_webhook %} converted to {% match message.actor %} with explicit Webhook and User arms. Fully qualified path crate::views::message_actor::MessageActor in the template means no import is needed in the modules of Template-deriving structs that include this partial (views/ws_fragments.rs, views/room.rs, views/dm.rs).
- routes/mod.rs, routes/dm.rs, routes/room.rs (4 sites), routes/ws.rs (3 sites): every MessageView construction updated to set actor: MessageActor::from_webhook_flag(...).
- tests/lc77_webhook_render_fixture.rs: updated to construct MessageView via the new shape; fixture files unchanged.

Verification: just check + clippy clean. All 50 lets-chat-server test binaries pass under both default and saas feature sets.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>