diff --git a/CHANGELOG.md b/CHANGELOG.md index 48e845b..ad7f1e4 100644 --- a/CHANGELOG.md +++ b/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". diff --git a/src/main.rs b/src/main.rs index 6647ccf..67dd353 100644 --- a/src/main.rs +++ b/src/main.rs @@ -794,38 +794,54 @@ async fn add( category: Option, list: bool, ) -> Result<()> { - let mut cfg = ctx.edit_cfg().await; - let url = &feed::find_feed(&ctx.client, &feed::expand_input(url)).await?; - // Includes feeds derived from an OPML, or the same show could be added twice. - if let Some(existing) = subscriptions(ctx).await?.iter().find(|s| feed::same_feed(&s.cfg.url, url)) { - // Listing a feed the catalogue already has, or giving it a category, is the point of - // asking again: it changes those, and nothing else. - if let Some(f) = cfg.feeds.get_mut(&existing.id) - && (list || category.is_some()) - { + let url = feed::find_feed(&ctx.client, &feed::expand_input(url)).await?; + let (id, existed) = add_to_catalogue(ctx, &url, folder, keywords).await?; + // Listing a feed the catalogue already has, or giving it a category, is the point of asking + // again: it changes those, and nothing else. A feed an OPML lists has no entry to change. + if list || category.is_some() { + let mut cfg = ctx.edit_cfg().await; + if let Some(f) = cfg.feeds.get_mut(&id) { 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); - return Ok(()); + if existed { + 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 let Some(f) = cfg.feeds.get_mut(&id) { - f.category = category; - f.listed = list; + if existed { + anyhow::bail!("already subscribed as {id:?}"); } - 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, + keywords: Vec, +) -> 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, diff --git a/src/web.rs b/src/web.rs index 786fff2..94cbaf0 100644 --- a/src/web.rs +++ b/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? - .into_iter() - .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?; + let (id, existed) = crate::add_to_catalogue(&state.ctx, &url, body.folder, body.keywords).await?; + let already = existed && state.ctx.db.subscription(user.id, &id).await?.is_some(); 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.