Add manual feed subscription #3

Merged
claude-bot merged 4 commits from worktree-manual-feed-subscribe into main 2026-08-21 14:54:28 +00:00
Collaborator

Summary

  • Adds a URL input + Subscribe button (via dx components add button input) to the reader UI
  • New subscribe_feed server function upserts the feed row and does an immediate first fetch, so the reader isn't empty until the next scheduled poll
  • feedsignal_feeds::fetch_feed now also returns the feed's parsed title, used to replace the URL placeholder once fetched

Test plan

  • cargo check/cargo test across native crates and both feedsignal-web feature sets (server + wasm32)
  • Manually verified via dx serve: subscribed to a real feed URL, saw "Subscribed — pulled in N article(s)." and the article list populate

Known follow-up (out of scope here)

"Not relevant" doesn't disable itself or remove the article from the list after dismissal — pre-existing gap, worth its own branch.

## Summary - Adds a URL input + Subscribe button (via `dx components add button input`) to the reader UI - New `subscribe_feed` server function upserts the feed row and does an immediate first fetch, so the reader isn't empty until the next scheduled poll - `feedsignal_feeds::fetch_feed` now also returns the feed's parsed title, used to replace the URL placeholder once fetched ## Test plan - [x] `cargo check`/`cargo test` across native crates and both `feedsignal-web` feature sets (server + wasm32) - [x] Manually verified via `dx serve`: subscribed to a real feed URL, saw "Subscribed — pulled in N article(s)." and the article list populate ## Known follow-up (out of scope here) "Not relevant" doesn't disable itself or remove the article from the list after dismissal — pre-existing gap, worth its own branch.
claude-bot added 1 commit 2026-08-21 13:16:40 +00:00
Add manual feed subscription
Some checks failed
CI / check (pull_request) Failing after 32s
CI / test (pull_request) Has been skipped
CI / audit (pull_request) Has been skipped
d40c90552c
Adds a URL input + Subscribe button (dx-components Input/Button) to the
UI, wired to a new subscribe_feed server function that upserts the feed
row and does an immediate first fetch so the reader isn't empty until
the next scheduled poll. feedsignal_feeds::fetch_feed now also returns
the feed's title, used to replace the URL placeholder once fetched.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJJQb2DWZZwQ1yPiaAQQoY
schaefera reviewed 2026-08-21 13:21:02 +00:00
@ -40,0 +73,4 @@
rsx! {
form {
class: "subscribe-form",
onsubmit: move |ev| {
Owner

Would indicate ev's type here for clarity as well

Would indicate ev's type here for clarity as well
schaefera marked this conversation as resolved
schaefera reviewed 2026-08-21 13:21:07 +00:00
@ -9,3 +10,4 @@
let bytes = reqwest::get(feed_url).await?.bytes().await?;
let parsed = feed_rs::parser::parse(&bytes[..])?;
let title = parsed
Owner

Rename to feed_title for clarity of purpose

Rename to `feed_title` for clarity of purpose
schaefera marked this conversation as resolved
schaefera reviewed 2026-08-21 13:21:46 +00:00
@ -40,0 +52,4 @@
let submit = move |_| {
let feed_url = url.read().clone();
if feed_url.trim().is_empty() {
Owner

Good guard, would be also nice if the submit button wasn't clickable if the feed_url text box was empty

Good guard, would be also nice if the submit button wasn't clickable if the feed_url text box was empty
schaefera marked this conversation as resolved
schaefera reviewed 2026-08-21 13:23:45 +00:00
@ -12,0 +14,4 @@
.title
.as_ref()
.map(|t| t.content.clone())
.filter(|t| !t.trim().is_empty())
Owner

Any way to invert this logic so the check is positive instead of negative? Would help grok it faster. Not major either way.

Any way to invert this logic so the check is positive instead of negative? Would help grok it faster. Not major either way.
schaefera marked this conversation as resolved
schaefera reviewed 2026-08-21 13:26:09 +00:00
@ -71,0 +72,4 @@
/// 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.
pub async fn subscribe_feed_impl(db: Db, url: String) -> Result<usize> {
Owner

This feels pseudo duplicated compared to some of the earlier code. What is the reason for both subscription-centric methods existing, just for my own understanding?

This feels pseudo duplicated compared to some of the earlier code. What is the reason for both subscription-centric methods existing, just for my own understanding?
schaefera marked this conversation as resolved
schaefera reviewed 2026-08-21 13:27:55 +00:00
@ -71,0 +74,4 @@
/// articles pulled in on this fetch.
pub async fn subscribe_feed_impl(db: Db, url: String) -> Result<usize> {
let url = url.trim();
anyhow::ensure!(!url.is_empty(), "feed URL is required");
Owner

Wouldn't this make sense to have before the trim with a is_blank() style check? Aka check if it's not just pure whitespace?

Wouldn't this make sense to have before the trim with a is_blank() style check? Aka check if it's not just pure whitespace?
schaefera marked this conversation as resolved
schaefera reviewed 2026-08-21 13:28:41 +00:00
@ -71,0 +76,4 @@
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
Owner

What if the url is correctly formed but otherwise points to nothing or a feed which no longer exists? Is this case handled?

What if the url is correctly formed but otherwise points to nothing or a feed which no longer exists? Is this case handled?
schaefera marked this conversation as resolved
schaefera requested changes 2026-08-21 13:29:42 +00:00
Dismissed
schaefera left a comment
Owner

Some aspects are unclear to me.

Also, more tests are needed.

We also need to do some thinking about the module architecture and how / whether an object oriented approach (not 100%, but more) might be sensible here.

Some aspects are unclear to me. Also, more tests are needed. We also need to do some thinking about the module architecture and how / whether an object oriented approach (not 100%, but more) might be sensible here.
schaefera added 1 commit 2026-08-21 13:32:44 +00:00
Address review feedback on manual feed subscription
Some checks failed
CI / check (pull_request) Failing after 16s
CI / test (pull_request) Has been skipped
CI / audit (pull_request) Has been skipped
8d316408b8
- Annotate the form onsubmit event type for clarity
- Disable the Subscribe button while the URL field is blank
- Rename title -> feed_title and invert the blank-title filter into a
  positively-named has_content helper, with a unit test
- Guard fetch_feed with error_for_status() so a well-formed URL that
  points at nothing (404/5xx) surfaces a clear error instead of an
  opaque feed-rs parse failure
- Collapse subscribe_feed_impl's two upsert_feed writes into one: fetch
  under a throwaway id first, then let upsert_feed be the single source
  of truth for the real feed id (existing id on re-subscribe, fresh
  otherwise), remapping fetched articles onto it before inserting. This
  also means a failed subscribe no longer leaves a placeholder row behind.
- Move the blank-URL check ahead of trimming

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJJQb2DWZZwQ1yPiaAQQoY
schaefera reviewed 2026-08-21 13:37:58 +00:00
@ -54,0 +77,4 @@
/// 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() {
Owner

Make three separate unit tests instead of one so we know which case breaks if a regression occurs.

Make three separate unit tests instead of one so we know which case breaks if a regression occurs.
schaefera marked this conversation as resolved
schaefera added 1 commit 2026-08-21 13:38:55 +00:00
Split has_content test into three cases per reviewer request
Some checks failed
CI / check (pull_request) Failing after 34s
CI / test (pull_request) Has been skipped
CI / audit (pull_request) Has been skipped
e621720e47
Separate tests for empty/whitespace/non-blank input so a regression
points at the exact case that broke instead of a single bundled test.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJJQb2DWZZwQ1yPiaAQQoY
schaefera added 1 commit 2026-08-21 13:41:20 +00:00
Fix CI: cargo fmt and clippy -D warnings failures
All checks were successful
CI / check (pull_request) Successful in 6m40s
CI / test (pull_request) Successful in 3m18s
CI / audit (pull_request) Successful in 11s
f28ed7b758
- cargo fmt: reformat a match arm in app.rs and drop trailing blank
  lines the dx-components generator left in two mod.rs files
- clippy -D warnings (feedsignal-web, server feature): the generated
  ButtonVariant/ButtonSize enums have variants this app doesn't use yet
  (they're part of the component library's API surface, not dead code
  in the ordinary sense) — clippy's dead_code lint was failing the
  build under -D warnings, fixed with a scoped #[allow(dead_code)]

Verified all three CI jobs locally: cargo fmt --check, clippy for
native crates + both feedsignal-web feature sets, cargo test
--workspace --exclude feedsignal-web, and cargo audit.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJJQb2DWZZwQ1yPiaAQQoY
schaefera approved these changes 2026-08-21 14:54:05 +00:00
schaefera left a comment
Owner

lgtm

lgtm
claude-bot merged commit fa9876a1b4 into main 2026-08-21 14:54:28 +00:00
claude-bot deleted branch worktree-manual-feed-subscribe 2026-08-21 14:54:28 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: schaefera/feedsignal#3
No description provided.