Add a feed through one function, from the CLI and the page alike (#161)

`ipx add` and the page's Add feed were the same steps written twice: find the feed, look for it in
the catalogue by same_feed, add_one, store. They had drifted, and #153's lock had to be put into
both by hand. add_to_catalogue() now does the shared part, under the lock, and answers the id and
whether the feed was there already; each caller still finds the feed itself, so the page can call a
bad address the caller's mistake (400), and then does its own part: the CLI lists or categorises,
the page subscribes and scans. add_one is private to it now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- `ipx add` and Add feed in the page add a feed the same way, through one piece of code.
 - Glass's top bar is solid on a phone, in the colour its glass showed, so the strip above it on an iPhone can match it.
 - A pinned item or feed's pin button is filled with the theme's accent, in the reader, on the feed's page and in the toolbar, so pinned no longer looks like not pinned.
 - A feed's page in the Directory says how many people here subscribe only when someone does, without "Nobody here subscribes yet".

src/main.rs

@@ -794,38 +794,54 @@ async fn add(
     category: Option<String>,
     list: bool,
 ) -> Result<()> {
-    let mut cfg = ctx.edit_cfg().await;
+    let url = feed::find_feed(&ctx.client, &feed::expand_input(url)).await?;
-    let url = &feed::find_feed(&ctx.client, &feed::expand_input(url)).await?;
+    let (id, existed) = add_to_catalogue(ctx, &url, folder, keywords).await?;
-    // Includes feeds derived from an OPML, or the same show could be added twice.
+    // Listing a feed the catalogue already has, or giving it a category, is the point of asking
-    if let Some(existing) = subscriptions(ctx).await?.iter().find(|s| feed::same_feed(&s.cfg.url, url)) {
+    // again: it changes those, and nothing else. A feed an OPML lists has no entry to change.
-        // Listing a feed the catalogue already has, or giving it a category, is the point of
+    if list || category.is_some() {
-        // asking again: it changes those, and nothing else.
+        let mut cfg = ctx.edit_cfg().await;
-        if let Some(f) = cfg.feeds.get_mut(&existing.id)
+        if let Some(f) = cfg.feeds.get_mut(&id) {
-            && (list || category.is_some())
-        {
             f.listed |= list;
             if category.is_some() {
                 f.category = category;
             }
             ctx.store_cfg(cfg).await?;
-            println!("{} is already in the catalogue; updated its Directory listing", existing.id);
+            if existed {
-            return Ok(());
+                println!("{id} is already in the catalogue; updated its Directory listing");
+                return Ok(());
+            }
         }
-        anyhow::bail!("already subscribed as {:?}", existing.id);
     }
-    let id = add_one(ctx, &mut cfg, url, folder, keywords).await?;
+    if existed {
-    if let Some(f) = cfg.feeds.get_mut(&id) {
+        anyhow::bail!("already subscribed as {id:?}");
-        f.category = category;
-        f.listed = list;
     }
-    ctx.store_cfg(cfg).await?;
     println!("added {id}{}", if list { ", listed in the Directory" } else { "" });
     Ok(())
 }
 
+/// Adds the feed at `url` to the catalogue unless it is there already, a feed an OPML lists
+/// included, so one show is never added twice: its id, and whether it was there. For `ipx add`
+/// and the page's Add feed alike, which were the same steps written twice (#161); each finds the
+/// feed first, to say its own way why an address is not one.
+pub async fn add_to_catalogue(
+    ctx: &Ctx,
+    url: &str,
+    folder: Option<String>,
+    keywords: Vec<String>,
+) -> Result<(String, bool)> {
+    // Taken before the duplicate check, so two people adding the same feed at once add it once.
+    let mut cfg = ctx.edit_cfg().await;
+    if let Some(existing) = subscriptions(ctx).await?.into_iter().find(|s| feed::same_feed(&s.cfg.url, url)) {
+        return Ok((existing.id, true));
+    }
+    let id = add_one(ctx, &mut cfg, url, folder, keywords).await?;
+    ctx.store_cfg(cfg).await?;
+    Ok((id, false))
+}
+
 /// Returns the new feed id. Adding checks the address is a feed first (`feed::find_feed`); this
 /// still names one it cannot read from its URL rather than failing.
-pub async fn add_one(
+async fn add_one(
     ctx: &Ctx,
     cfg: &mut config::Config,
     url: &str,

src/web.rs

@@ -1492,30 +1492,16 @@ async fn add_feed(
     let url = crate::feed::find_feed(&state.ctx.client, &crate::feed::expand_input(&body.url))
         .await
         .map_err(|e| ApiError::bad_request(format!("{e:#}")))?;
-    // Taken before the duplicate check, so two people adding the same feed at once add it once.
-    let mut cfg = state.ctx.edit_cfg().await;
     // Someone else may already have it. Then adding costs nothing: no second fetch, no
     // second copy on disk, just another name against the same feed.
-    if let Some(existing) = crate::subscriptions(&state.ctx).await?
+    let (id, existed) = crate::add_to_catalogue(&state.ctx, &url, body.folder, body.keywords).await?;
-        .into_iter()
+    let already = existed && state.ctx.db.subscription(user.id, &id).await?.is_some();
-        .find(|s| crate::feed::same_feed(&s.cfg.url, &url))
-    {
-        let already = state.ctx.db.subscription(user.id, &existing.id).await?.is_some();
-        state.ctx.db.subscribe(user.id, &existing.id).await?;
-        if !already {
-            explicit_on_add(&state, user.id, &existing.id, body.allow_explicit).await?;
-        }
-        scan_soon(&state, Some(existing.id.clone())).await;
-        return Ok(Json(
-            serde_json::json!({ "id": existing.id, "existing": already }),
-        ));
-    }
-    let id = crate::add_one(&state.ctx, &mut cfg, &url, body.folder, body.keywords).await?;
-    state.ctx.store_cfg(cfg).await?;
     state.ctx.db.subscribe(user.id, &id).await?;
-    explicit_on_add(&state, user.id, &id, body.allow_explicit).await?;
+    if !already {
+        explicit_on_add(&state, user.id, &id, body.allow_explicit).await?;
+    }
     scan_soon(&state, Some(id.clone())).await;
-    Ok(Json(serde_json::json!({ "id": id, "existing": false })))
+    Ok(Json(serde_json::json!({ "id": id, "existing": already })))
 }
 
 /// Queues a scan, so a feed just added shows its items without anyone pressing Scan now.