diff --git a/crates/ipx/CHANGELOG.md b/crates/ipx/CHANGELOG.md index da5fa73..f60de0c 100644 --- a/crates/ipx/CHANGELOG.md +++ b/crates/ipx/CHANGELOG.md @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- `GET /api/feeds/{id}/entries` for an OPML subscription or a Patreon creator lists the items of the shows in it that you subscribe to, together, where it listed nothing (#283). - A TypeSafe key is checked with TypeSafe before it is saved, and one it refuses is not saved (#271). - A new Signal reads the last week instead of the last 30 days, unless an admin sets otherwise (#276). - Editing a Signal's name, words or description asks again only about what it had picked out, and about new items as they come, instead of the whole window again (#276). diff --git a/crates/ipx/src/db.rs b/crates/ipx/src/db.rs index 6907c27..d5e8765 100644 --- a/crates/ipx/src/db.rs +++ b/crates/ipx/src/db.rs @@ -1151,15 +1151,19 @@ impl Args { fn entries_from( a: &mut Args, user_id: i64, - feed_id: Option<&str>, + feeds: Option<&[&str]>, filter: Filter, search: Option<&str>, guid: Option<&str>, signal: Option, ) -> String { let user = a.p(user_id); - let mut scope = match feed_id { - Some(f) => format!("e.feed_id = {}", a.p(f)), + let mut scope = match feeds { + // One feed as before, so its page keeps its plan: an OR with the shows' list, or an IN + // over a subquery, took clarkesworld's from 10 ms to 26 ms or a second (#283). + Some([f]) => format!("e.feed_id = {}", a.p(*f)), + Some([]) => "false".into(), + Some(fs) => format!("e.feed_id IN ({})", fs.iter().map(|f| a.p(*f)).collect::>().join(", ")), None => format!("e.feed_id IN (SELECT feed_id FROM subscriptions WHERE user_id = {user})"), }; if let Some(id) = signal { @@ -1309,7 +1313,37 @@ impl Db { limit: i64, order: &str, ) -> Result> { - self.entries_where(user_id, feed_id, filter, search, None, None, offset, limit, order).await + let feeds = feed_id.as_ref().map(std::slice::from_ref); + self.entries_where(user_id, feeds, filter, search, None, None, offset, limit, order).await + } + + /// As `entries_in`, for several feeds together (an OPML's shows, #283), or every subscribed + /// feed when None. + #[allow(clippy::too_many_arguments)] + pub async fn entries_of( + &self, + user_id: i64, + feeds: Option<&[&str]>, + filter: Filter, + search: Option<&str>, + offset: i64, + limit: i64, + order: &str, + ) -> Result> { + self.entries_where(user_id, feeds, filter, search, None, None, offset, limit, order).await + } + + /// The shows of an OPML (or a Patreon creator) this person subscribes to (#283). + pub async fn group_members(&self, user_id: i64, group: &str) -> Result> { + self.rows( + "SELECT gs.feed_id FROM subscriptions gs JOIN feeds g ON g.id = gs.feed_id + WHERE gs.user_id = $1 AND g.group_id = $2", + vec![user_id.into(), group.into()], + ) + .await? + .iter() + .map(|r| Ok(r.try_get("", "feed_id")?)) + .collect() } /// A Signal's page of entries, as `entries_in` gives All Subscriptions'. The caller has made @@ -1340,14 +1374,17 @@ impl Db { /// plays in changes (DeadAir, #171). pub async fn entry(&self, user_id: i64, feed_id: &str, guid: &str) -> Result> { let order = order_sql("published", "desc"); - Ok(self.entries_where(user_id, Some(feed_id), Filter::All, None, Some(guid), None, 0, 1, &order).await?.pop()) + Ok(self + .entries_where(user_id, Some(&[feed_id]), Filter::All, None, Some(guid), None, 0, 1, &order) + .await? + .pop()) } #[allow(clippy::too_many_arguments)] async fn entries_where( &self, user_id: i64, - feed_id: Option<&str>, + feeds: Option<&[&str]>, filter: Filter, search: Option<&str>, guid: Option<&str>, @@ -1357,7 +1394,7 @@ impl Db { order: &str, ) -> Result> { let mut a = Args::default(); - let from = entries_from(&mut a, user_id, feed_id, filter, search, guid, signal); + let from = entries_from(&mut a, user_id, feeds, filter, search, guid, signal); // The page's keys first, then its rows: the sort then needs only `entries_by_time`, not // every item's row, and the show notes are read for the page alone (#188). let (limit, offset) = (a.p(limit), a.p(offset)); @@ -1421,10 +1458,20 @@ impl Db { feed_id: Option<&str>, filter: Filter, search: Option<&str>, + ) -> Result { + self.count_of(user_id, feed_id.as_ref().map(std::slice::from_ref), filter, search).await + } + + /// As `count_in`, for several feeds together, or every subscribed feed when None. + pub async fn count_of( + &self, + user_id: i64, + feeds: Option<&[&str]>, + filter: Filter, + search: Option<&str>, ) -> Result { let mut a = Args::default(); - let sql = - format!("SELECT count(*) AS n {}", entries_from(&mut a, user_id, feed_id, filter, search, None, None)); + let sql = format!("SELECT count(*) AS n {}", entries_from(&mut a, user_id, feeds, filter, search, None, None)); let r = self.rows(&sql, a.0).await?; Ok(r.first().context("count(*) always returns a row")?.try_get("", "n")?) } @@ -3623,6 +3670,32 @@ mod tests { assert_eq!(db.subscribers("g", None).await.unwrap()[0].blocked, ["gardening"]); } + #[tokio::test] + async fn an_opml_lists_its_shows_items_together_for_whoever_subscribes() { + let db = Db::memory().await.unwrap(); + db.exec_for_test( + "INSERT INTO users (id, name, is_admin) VALUES (1,'a',true), (2,'b',false); + INSERT INTO feeds (id, url, group_id, managed) VALUES ('a','u1','o',true), ('b','u2','o',true), ('c','u3',NULL,false); + INSERT INTO subscriptions (user_id, feed_id) VALUES (1,'o'), (1,'a'), (1,'b'), (1,'c'), (2,'a'); + INSERT INTO entries (feed_id, guid, title, first_seen) VALUES + ('a','1','A one',1), ('b','2','B two',2), ('c','3','C three',3);", + ) + .await + .unwrap(); + let mut shows = db.group_members(1, "o").await.unwrap(); + shows.sort(); + assert_eq!(shows, ["a", "b"], "the OPML's shows, not a feed outside it"); + assert_eq!(db.group_members(2, "o").await.unwrap(), ["a"], "only the shows this person has"); + + let ids = ["a", "b", "o"]; + let order = order_sql("published", "desc"); + let rows = db.entries_of(1, Some(&ids), Filter::All, None, 0, 50, &order).await.unwrap(); + let titles: Vec<_> = rows.iter().map(|r| r.title.clone().unwrap()).collect(); + assert_eq!(titles, ["B two", "A one"], "both shows' items, newest first"); + assert_eq!(db.count_of(1, Some(&ids), Filter::All, None).await.unwrap(), 2); + assert_eq!(db.count_of(1, Some(&[]), Filter::All, None).await.unwrap(), 0); + } + #[tokio::test] async fn pending_takes_the_latest_episodes_first() { // A cap of 3 must mean the three newest, not the three recorded first. diff --git a/crates/ipx/src/web.rs b/crates/ipx/src/web.rs index 1e72aba..6bc162c 100644 --- a/crates/ipx/src/web.rs +++ b/crates/ipx/src/web.rs @@ -1459,7 +1459,12 @@ async fn entries( Query(page): Query, ) -> Result, ApiError> { subscribed(&state, &user, &id).await?; - entry_page(&state, user.id, Some(&id), None, &page).await + // An OPML's own row has no items: its page is those of its shows, together, as All + // Subscriptions lists every feed's (#283). + let mut ids = state.ctx.db.group_members(user.id, &id).await?; + ids.push(id); + let ids: Vec<&str> = ids.iter().map(String::as_str).collect(); + entry_page(&state, user.id, Some(&ids[..]), None, &page).await } /// A feed's items and files are for the people who subscribe to it (#129). Anyone signed in @@ -1483,11 +1488,11 @@ async fn all_entries( entry_page(&state, user.id, None, None, &page).await } -/// One feed's page of items, or every subscribed feed's when `feed` is None, or a Signal's. +/// Some feeds' page of items, or every subscribed feed's when `feeds` is None, or a Signal's. async fn entry_page( state: &WebState, user_id: i64, - feed: Option<&str>, + feeds: Option<&[&str]>, signal: Option, page: &Page, ) -> Result, ApiError> { @@ -1504,7 +1509,7 @@ async fn entry_page( .await? } None => { - db.entries_in(user_id, feed, filter, search, page.offset, limit, &order) + db.entries_of(user_id, feeds, filter, search, page.offset, limit, &order) .instrument(step("entries_in")) .await? } @@ -1520,7 +1525,7 @@ async fn entry_page( crate::deadair::for_reader(&state.ctx, user_id, &mut rows).instrument(step("deadair")).await?; let total = match signal { Some(id) => db.signal_count(user_id, id, filter, search).instrument(step("count_in")).await?, - None => db.count_in(user_id, feed, filter, search).instrument(step("count_in")).await?, + None => db.count_of(user_id, feeds, filter, search).instrument(step("count_in")).await?, }; Ok(Json(EntryPage { total, entries: rows })) } diff --git a/docs/signals.md b/docs/signals.md index e5db252..3dfba5f 100644 --- a/docs/signals.md +++ b/docs/signals.md @@ -30,7 +30,7 @@ that has not heard about the item yet. **Auto** asks Jev to choose (#263), and a Signal made without an icon gets Jev's pick by itself, a moment after it is made. Every Signal's tile is in the accent tint the folder has. - **Each Signal's row** says "12 items · 3 unread", as a feed's row counts its own. -- **The folder's page** lists the Signals, as an OPML's page lists its feeds, with the + for a new one. +- **The folder's page** lists the Signals, with the + for a new one. - **A Signal's page** is an ordinary item list: All, Unread, Downloaded and Pinned, search and sort. Its header says what it listens for (the sentence, or the words), and, on people's own keys, that nothing is picked out without one. Edit changes the name, the words or the sentence; Delete removes the Signal diff --git a/site/api.mjs b/site/api.mjs index 7bdf4ec..d71766c 100644 --- a/site/api.mjs +++ b/site/api.mjs @@ -168,7 +168,7 @@ export const ipx = { items: [ { method: 'GET', path: '/api/feeds/{id}/entries', slug: 'get-feed-entries', - summary: 'A page of one feed’s episodes, newest first unless sorted.', + summary: 'A page of one feed’s episodes, newest first unless sorted. For an OPML list or a Patreon creator, those of the shows in it you subscribe to, together.', pathParams: [['id', 'String', 'The feed’s id.']], query: 'Page', returns: 'EntryPage', errors: [NOT_SUBSCRIBED], example: ipxCurl('GET', '/api/feeds/99-invisible/entries?filter=unread&limit=10'), diff --git a/tests/page-smoke.js b/tests/page-smoke.js index 8a810b1..65bb2ef 100644 --- a/tests/page-smoke.js +++ b/tests/page-smoke.js @@ -54,7 +54,7 @@ const drive = PAGE === 'admin.html' ? [ ['keysModal', () => ctx.keysModal()], // `const S` is not reachable from here: top-level const/let do not become properties // of a vm context the way var and function declarations do. - ['renderGroup', () => ctx.renderGroup(feed, [{ ...feed, id: 'child', group: 'f', orphaned: true }])], + ['groupHead', () => ctx.groupHead(feed, [{ ...feed, id: 'child', group: 'f', orphaned: true }])], ]; for (const [name, fn] of drive) { try { diff --git a/tests/ui/app.spec.js b/tests/ui/app.spec.js index 1c1bcf0..38a840f 100644 --- a/tests/ui/app.spec.js +++ b/tests/ui/app.spec.js @@ -447,9 +447,12 @@ test('an OPML subscription is a collapsible folder', async ({ page }) => { // Scoped to the sidebar: the name also appears as the page heading once selected. await expect(page.locator('#feedlist').getByText('Grouped Show')).toBeVisible(); - // The subscription's own page lists what is inside it. + // The subscription's own page lists the items of the shows inside it, together (#283). await page.locator('.feed', { hasText: 'Test Subscriptions' }).first().click(); - await expect(page.locator('.childrow')).toHaveCount(2); + await page.locator('#content .tabs button[data-f="all"]').click(); + await expect(page.locator('.ep', { hasText: 'Aardvark Ep' })).toBeVisible(); + await expect(page.locator('.ep', { hasText: 'Grouped Ep' })).toBeVisible(); + await expect(page.locator('.childrow')).toHaveCount(0); }); test('inside an OPML, feeds with unread items are listed first', async ({ page }) => { @@ -482,8 +485,11 @@ test('inside an OPML, feeds with unread items are listed first', async ({ page } await page.reload(); const want = ['Grouped Show', 'Aardvark Radio']; await expect(page.locator('#feedlist .feed.child b')).toHaveText(want, { timeout: 20_000 }); + // The OPML's Unread tab has the unread show's item and not the read one's (#283). await page.locator('.feed', { hasText: 'Test Subscriptions' }).first().click(); - await expect(page.locator('.childrow b')).toHaveText(want); + await page.locator('#content .tabs button[data-f="unread"]').click(); + await expect(page.locator('.ep', { hasText: 'Grouped Ep' })).toBeVisible(); + await expect(page.locator('.ep', { hasText: 'Aardvark Ep' })).toHaveCount(0); }); test('a show inside an OPML can be kept, and only it', async ({ page }) => { @@ -511,7 +517,7 @@ test('a show inside an OPML can be kept, and only it', async ({ page }) => { // The OPML itself has no Keep. await page.locator('.feed', { hasText: 'Test Subscriptions' }).first().click(); - await expect(page.locator('.childrow').first()).toBeVisible(); + await expect(page.locator('#content h2')).toHaveText('Test Subscriptions'); await expect(page.locator('#content [data-a="keep"]')).toHaveCount(0); }); diff --git a/web/CHANGELOG.md b/web/CHANGELOG.md index f779214..93828fd 100644 --- a/web/CHANGELOG.md +++ b/web/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- An OPML subscription's page lists the items of all its shows together, newest first, with the same tabs, search and sort as a show's, in place of a list of the shows; the shows are still in the folder in the feed list (#283). - Unless the server pays for them, Signals are turned on in Settings with your own TypeSafe key, and the Signals folder shows only once they are on (#271). - A Signal's dialog says how far back it reads, and that an edit reads again only what was picked out (#276). diff --git a/web/src/events.ts b/web/src/events.ts index d6254c4..979eef2 100644 --- a/web/src/events.ts +++ b/web/src/events.ts @@ -62,7 +62,7 @@ function connect(){ else if(ev.ev==='feed_done'){ setScanning(ev.feed,false); if(!mine(ev.feed)) return; - if(ev.feed===S.feed||S.feed===':all') refreshEntries(); + if(ev.feed===S.feed||S.feed===':all'||S.feeds.some(f=>f.id===ev.feed&&f.group===S.feed)) refreshEntries(); } // No toast: a scan of every feed raised one per failure, to everyone. The feed list's // red ! marks the feed instead, and its page says why. diff --git a/web/src/feedpage.ts b/web/src/feedpage.ts index afe5d46..bca2e3d 100644 --- a/web/src/feedpage.ts +++ b/web/src/feedpage.ts @@ -14,9 +14,8 @@ function renderFeed(){ const name=f?(f.title||f.id):sg?sg.name:v.title; $('#epSearch').placeholder=`Search ${name}…`; const kids=f?S.feeds.filter(c=>c.group===f.id):[]; - if(kids.length){ box.classList.add('plain'); renderGroup(f!,kids); return; } const unreadAll=S.feeds.reduce((n,x)=>n+(x.unread||0),0); - box.innerHTML = (f ? ` + box.innerHTML = (f&&kids.length ? groupHead(f,kids) : f ? `
${artHTML(f.image,name)}
@@ -61,7 +60,8 @@ function renderFeed(){ const pane=document.createElement('div'); pane.id='split'; - if(f) pane.className='one'; + // One feed's list leaves out the feed column; an OPML's shows are told apart by it. + if(f&&!kids.length) pane.className='one'; pane.innerHTML='
'+sortHead()+ '
'; $$('.ephead [data-sort]',pane).forEach(b=>b.onclick=()=>sortBy(b.dataset.sort!)); @@ -100,26 +100,25 @@ function sortBy(col: string){ S.offset=0; renderFeed(); loadEntries(); } -/// An OPML subscription's page lists the feeds inside it rather than items, but keeps -/// every action a normal feed has -- it is still an ordinary feed entry underneath. +/// An OPML subscription's page lists the items of the shows inside it together, as All +/// Subscriptions does every feed's (#283), under the header a feed has, its buttons in the +/// same places: it is still an ordinary feed entry underneath. // A Patreon creator split into its shows is drawn like an OPML, and named for what it is. const isPatreon=(f: {url: string}|null)=>/patreon\.com\//.test(f&&f.url||''); -function renderGroup(f: Feed,kids: Feed[]){ +function groupHead(f: Feed,kids: Feed[]){ const unread=kids.reduce((n,c)=>n+c.unread,0); const saved=kids.reduce((n,c)=>n+c.downloaded,0); const gone=kids.filter(c=>c.orphaned).length; - $('#count').textContent=`${f.title||f.id}: ${kids.length} feed${kids.length===1?'':'s'}, ${unread} unread`; - // The same header as a feed's, buttons in the same places: it is a feed underneath. - $('#content').innerHTML = ` + return `
${folderArt(f,kids)}

${esc(f.title||f.id)}

${isPatreon(f)?'Patreon creator':'OPML subscription'} - · ${plural(kids.length,'feed')}, ${unread} unread, ${saved} downloaded · checked ${ago(f.last_checked)}
+ · ${plural(kids.length,'show')}, ${unread} unread, ${saved} downloaded · checked ${ago(f.last_checked)}
${failBannerHTML(f)} - ${gone?`
${gone} feed${gone===1?' is':'s are'} no longer + ${gone?`
${gone} show${gone===1?' is':'s are'} no longer listed but kept because ${gone===1?'it has':'they have'} downloads.
`:''}
@@ -130,36 +129,7 @@ function renderGroup(f: Feed,kids: Feed[]){
-
-
- - ${esc(f.url)} -
-
`; - $$('#content .acts .btn').forEach(b=>b.onclick=()=>feedAction(b.dataset.a!,f)); - wireFailBanner($('#content'),f); - const draw=()=>{ - const q=($('#kidSearch').value||'').trim().toLowerCase(); - const box=$('#kidlist'); box.innerHTML=''; - const rows=kids.filter(c=>!q||(c.title||c.id).toLowerCase().includes(q)).sort(unreadFirst); - if(!rows.length){ box.innerHTML='

Nothing matches.

'; return; } - for(const c of rows){ - const el=document.createElement('div'); - el.className='childrow'; - el.innerHTML = artHTML(c.image,c.title||c.id)+ - `
${esc(c.title||c.id)}`+ - `${plural(c.entries,'item')} · ${c.downloaded} downloaded`+ - (c.failing?` · error` - :c.last_error?` · error`:'')+`
`+ - (c.orphaned?'Gone':'')+ - (c.failing?`Error`:'')+ - `${c.unread}`; - el.onclick=()=>selectFeed(c.id); - box.appendChild(el); - } - }; - $('#kidSearch').oninput=draw; - draw(); +
`; } /// Keep, for a show from an OPML or a Patreon creator: it stays yours, a show of its own, should diff --git a/web/src/items.ts b/web/src/items.ts index c95af5d..a077fcc 100644 --- a/web/src/items.ts +++ b/web/src/items.ts @@ -24,11 +24,9 @@ function keepList(key: string, r: ListPage){ /// Fetches a list's first page ahead of a click: when the page has loaded, and when the pointer /// reaches a feed or a filter. Not one kept already or on its way. function prefetch(feed: string, filter=S.filter){ - // Currently Listening and the Directory draw from their own address; a folder's page lists its - // feeds, not items. + // Currently Listening and the Directory draw from their own address. const url=VIEWS[feed]?.url; if(url) return prefetchPage(url); - if(S.feeds.some(c=>c.group===feed)) return; const key=listKey(feed,filter); if(lists.has(key)||fetching.has(key)) return; const got: Promise=api(listUrl(feed,filter)); @@ -51,8 +49,9 @@ const prefetchPage=(url: string)=>{ if(!pages.has(url)) fetchPage(url).catch(()= /// A feed changed (a scan, a read elsewhere): its kept lists, and every one of All Subscriptions, /// are fetched again when next wanted. function forgetLists(feed: string){ - // A Signal holds items of any feed. - for(const k of [...lists.keys()]) if(k.startsWith(feed+'\n')||k.startsWith(':all\n')||k.startsWith(':signal:')) lists.delete(k); + // A Signal holds items of any feed, and an OPML's list its shows' (#283). + const group=S.feeds.find(f=>f.id===feed)?.group; + for(const k of [...lists.keys()]) if(k.startsWith(feed+'\n')||k.startsWith(':all\n')||k.startsWith(':signal:')||(group&&k.startsWith(group+'\n'))) lists.delete(k); } /// What is likely to be opened next, fetched two at a time once the page is idle: All Subscriptions /// and the feeds with something unread, most first. @@ -125,7 +124,9 @@ function listLoading(box=$('#eps')){ function renderEntries(){ const box=$('#eps'); if(!box) return; const f=S.feeds.find(x=>x.id===S.feed), v=VIEWS[S.feed ?? ''], sg=signalOf(S.feed); - const unread=f?f.unread:sg?sg.unread:S.feeds.reduce((n,x)=>n+(x.unread||0),0); + // An OPML's own row counts nothing: its shows do. + const kids=f?S.feeds.filter(c=>c.group===f.id):[]; + const unread=kids.length?kids.reduce((n,x)=>n+x.unread,0):f?f.unread:sg?sg.unread:S.feeds.reduce((n,x)=>n+(x.unread||0),0); $('#count').textContent= `${f?(f.title||f.id):sg?sg.name:v?v.title:''}: ${S.total} item${S.total===1?'':'s'}, ${unread} unread`; const pane=$('#list'), top=pane?pane.scrollTop:0;