diff --git a/crates/feeds/src/lib.rs b/crates/feeds/src/lib.rs index cfb72d4..96b0d8b 100644 --- a/crates/feeds/src/lib.rs +++ b/crates/feeds/src/lib.rs @@ -7,14 +7,18 @@ use uuid::Uuid; /// entries. `feed-rs` normalizes both RSS and Atom into a common model so /// callers don't need to branch on feed type. pub async fn fetch_feed(feed_url: &str, feed_id: Uuid) -> Result<(String, Vec
)> { - let bytes = reqwest::get(feed_url).await?.bytes().await?; + // `.error_for_status()` turns a 404/5xx into a clear `Err` here rather + // than letting a non-feed error page fall through to a confusing + // feed-rs parse failure below. + let response = reqwest::get(feed_url).await?.error_for_status()?; + let bytes = response.bytes().await?; let parsed = feed_rs::parser::parse(&bytes[..])?; - let title = parsed + let feed_title = parsed .title .as_ref() .map(|t| t.content.clone()) - .filter(|t| !t.trim().is_empty()) + .filter(|t| has_content(t)) .unwrap_or_else(|| feed_url.to_string()); let articles = parsed @@ -47,7 +51,13 @@ pub async fn fetch_feed(feed_url: &str, feed_id: Uuid) -> Result<(String, Vec` rather than surface it as-is. +fn has_content(s: &str) -> bool { + !s.trim().is_empty() } /// Rough estimate from word count at ~200 wpm, used as the denominator for @@ -59,3 +69,17 @@ fn estimate_read_seconds(text: &str) -> u32 { let words = text.split_whitespace().count() as u32; ((words.max(1) as f32 / 200.0) * 60.0) as u32 } + +#[cfg(test)] +mod tests { + use super::*; + + /// A blank or whitespace-only feed title should be treated the same as + /// a missing one, not surfaced as an empty string in the UI. + #[test] + fn has_content_rejects_blank_strings() { + assert!(!has_content("")); + assert!(!has_content(" \n\t")); + assert!(has_content("Rust Blog")); + } +} diff --git a/crates/web/src/app.rs b/crates/web/src/app.rs index de3e831..8f0115d 100644 --- a/crates/web/src/app.rs +++ b/crates/web/src/app.rs @@ -73,7 +73,7 @@ fn SubscribeForm(on_subscribed: EventHandler<()>) -> Element { rsx! { form { class: "subscribe-form", - onsubmit: move |ev| { + onsubmit: move |ev: FormEvent| { ev.prevent_default(); submit(()); }, @@ -88,7 +88,7 @@ fn SubscribeForm(on_subscribed: EventHandler<()>) -> Element { Button { r#type: "submit", variant: ButtonVariant::Primary, - disabled: submitting(), + disabled: submitting() || url.read().trim().is_empty(), if submitting() { "Subscribing..." } else { diff --git a/crates/web/src/server.rs b/crates/web/src/server.rs index 9fcdd31..38bedfe 100644 --- a/crates/web/src/server.rs +++ b/crates/web/src/server.rs @@ -72,16 +72,24 @@ pub async fn list_ranked_articles_impl(db: Db) -> Result> { /// already subscribed) and does an immediate first fetch so the reader /// isn't empty until the next scheduled poll. Returns the number of /// articles pulled in on this fetch. +/// +/// Fetches under a throwaway id before writing anything, so there's exactly +/// one write to the `feeds` table (`upsert_feed` is the single source of +/// truth for the real id — the existing one on a re-subscribe, a fresh one +/// otherwise) and no half-registered row left behind if the fetch fails +/// (e.g. the URL is well-formed but points at nothing, or the feed no +/// longer exists — `fetch_feed` surfaces both as an `Err` via +/// `error_for_status`/feed-rs parse failure, which becomes the error +/// message shown in the subscribe form). pub async fn subscribe_feed_impl(db: Db, url: String) -> Result { + anyhow::ensure!(!url.trim().is_empty(), "feed URL is required"); let url = url.trim(); - anyhow::ensure!(!url.is_empty(), "feed URL is required"); - // Register under a placeholder title first so the feed exists even if - // the fetch below fails (e.g. transient network error) — it'll be - // picked up by a later poll once feed polling is wired in. - let feed_id = db.upsert_feed(url, url).await?; - let (title, articles) = feedsignal_feeds::fetch_feed(url, feed_id).await?; - db.upsert_feed(url, &title).await?; + let (title, mut articles) = feedsignal_feeds::fetch_feed(url, Uuid::new_v4()).await?; + let feed_id = db.upsert_feed(url, &title).await?; + for article in &mut articles { + article.feed_id = feed_id; + } let count = articles.len(); for article in &articles {