From 23f7ba7ddd969d0556f225be46213670c6d554f0 Mon Sep 17 00:00:00 2001 From: Luna Date: Sun, 31 May 2026 21:28:10 -0700 Subject: [PATCH] N-level folder nesting (1.2+) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Folders were flat โ€” one level under each platform. Tartube supports arbitrary nesting; now we do too. ## Schema Add `folders.parent_id INTEGER REFERENCES folders(id) ON DELETE CASCADE` via an idempotent migration (ALTER TABLE, swallow the duplicate-column error on already-migrated DBs). NULL = top-level. Cascade means deleting a parent removes its subtree's folder rows; member channels revert to Unfiled via the existing channel_assignments cascade. ## DB - FolderRecord gains parent_id (flows to both UIs via serde). - set_folder_parent(id, parent) reparents, with a cycle guard: walks the ancestor chain of the proposed parent and refuses if `id` appears (you can't nest a folder inside its own descendant). Self-parent also rejected. Returns a friendly SQLITE_CONSTRAINT error the web layer maps to 400. ## API POST /api/folders/:id/parent { parent_id } โ€” reparent or null for top. ## Web UI - Sidebar renders the folder tree recursively: each folder indents one step under its parent, member channels indent under their folder, and the count shown is the whole subtree. A seen-set guards against any malformed cycle in the data. - Folder manager gains a per-row "parent" dropdown; choices that would cycle (self + descendants) are excluded. ## Desktop UI - Channel-panel sidebar builds the same tree with an iterative DFS (depth-indented folder headers, subtree counts). - Folder manager gains a parent ComboBox per row with the same cycle-exclusion. Verified the migration runs against an existing real DB (parent_id added, no data loss). 1 new test covers nesting + both cycle cases; 80 pass. Co-Authored-By: Claude Opus 4.7 --- src/app.rs | 126 ++++++++++++++++++++++++++++++++++++++---- src/database.rs | 95 ++++++++++++++++++++++++++++++- src/web.rs | 28 ++++++++++ src/web_ui/index.html | 93 +++++++++++++++++++++++-------- 4 files changed, 309 insertions(+), 33 deletions(-) diff --git a/src/app.rs b/src/app.rs index a474299..d96bd4a 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1194,19 +1194,71 @@ impl App { // so folders render above platform sections. Channels with // a folder_id appear only in their folder; unfiled channels // still get the per-platform grouping. - enum SidebarItem { FolderHeader(String, usize), FolderManageBtn, PlatformHeader(Platform), Channel(usize) } + // FolderHeader carries a `depth` for indentation so the + // N-level nesting reads as a tree. + enum SidebarItem { FolderHeader(String, usize, usize), FolderManageBtn, PlatformHeader(Platform), Channel(usize, usize) } let mut items: Vec = Vec::new(); let has_any_folder_or_assignment = !self.folders.is_empty() || self.library.iter().any(|c| c.folder_id.is_some()); if has_any_folder_or_assignment { items.push(SidebarItem::FolderManageBtn); - for f in &self.folders { - let members: Vec = self.library.iter().enumerate() - .filter(|(_, c)| c.folder_id == Some(f.id)) + // Recursively emit a folder, its member channels, then + // its children. Count includes the whole subtree. + let folders = self.folders.clone(); + let library = &self.library; + let subtree_count = |fid: i64| -> usize { + // Iterative DFS counting members of fid + descendants. + let mut seen = std::collections::HashSet::new(); + let mut stack = vec![fid]; + let mut n = 0usize; + while let Some(cur) = stack.pop() { + if !seen.insert(cur) { continue; } + n += library.iter().filter(|c| c.folder_id == Some(cur)).count(); + for f in &folders { + if f.parent_id == Some(cur) { stack.push(f.id); } + } + } + n + }; + // Walk the tree depth-first. `emit` is iterative to avoid + // borrow-checker gymnastics with a recursive closure. + let mut seen = std::collections::HashSet::new(); + // Stack holds (folder_index_into_folders, depth). + let roots: Vec = folders.iter().enumerate() + .filter(|(_, f)| f.parent_id.is_none()) + .map(|(i, _)| i) + .collect(); + // Push roots in reverse so the first root pops first. + let mut stack: Vec<(usize, usize)> = + roots.into_iter().rev().map(|i| (i, 0usize)).collect(); + while let Some((fi, depth)) = stack.pop() { + let f = &folders[fi]; + if !seen.insert(f.id) { continue; } + items.push(SidebarItem::FolderHeader(f.name.clone(), subtree_count(f.id), depth)); + for (i, c) in library.iter().enumerate() { + if c.folder_id == Some(f.id) { + items.push(SidebarItem::Channel(i, depth + 1)); + } + } + // Push children (reverse for stable order). + let mut kids: Vec = folders.iter().enumerate() + .filter(|(_, c)| c.parent_id == Some(f.id)) .map(|(i, _)| i) .collect(); - items.push(SidebarItem::FolderHeader(f.name.clone(), members.len())); - for i in members { items.push(SidebarItem::Channel(i)); } + kids.reverse(); + for ki in kids { stack.push((ki, depth + 1)); } + } + // Orphans whose parent was deleted: render at root. + for (fi, f) in folders.iter().enumerate() { + if !seen.contains(&f.id) { + items.push(SidebarItem::FolderHeader(f.name.clone(), subtree_count(f.id), 0)); + for (i, c) in library.iter().enumerate() { + if c.folder_id == Some(f.id) { + items.push(SidebarItem::Channel(i, 1)); + } + } + let _ = fi; + } } } else { items.push(SidebarItem::FolderManageBtn); @@ -1218,7 +1270,8 @@ impl App { items.push(SidebarItem::PlatformHeader(ch.platform)); last_platform_marker = Some(ch.platform); } - items.push(SidebarItem::Channel(i)); + // Unfiled channels render at the flat (depth 0) indent. + items.push(SidebarItem::Channel(i, 0)); } for item in items { @@ -1231,9 +1284,11 @@ impl App { } }); } - SidebarItem::FolderHeader(name, count) => { + SidebarItem::FolderHeader(name, count, depth) => { + // Two spaces of base indent + 2 per nesting level. + let indent = " ".repeat(depth + 1); ui.label( - egui::RichText::new(format!(" {name} ({count})")) + egui::RichText::new(format!("{indent}๐Ÿ“ {name} ({count})")) .small() .weak(), ); @@ -1245,7 +1300,7 @@ impl App { .weak(), ); } - SidebarItem::Channel(i) => { + SidebarItem::Channel(i, _depth) => { let (name, total, has_playlists, size_bytes, channel_url, platform) = { let ch = &self.library[i]; // Prefer the stored `.source-url` over folder-name guessing so @@ -1961,6 +2016,8 @@ impl App { let mut to_rename: Option<(i64, String)> = None; let mut to_delete: Option<(i64, String, usize)> = None; let mut to_check: Option = None; + // (folder id, new parent or None) collected during render, applied after. + let mut to_reparent: Option<(i64, Option)> = None; egui::Window::new("๐Ÿ“ Manage folders") .open(&mut open) .collapsible(false) @@ -1971,6 +2028,20 @@ impl App { ui.label(egui::RichText::new("No folders yet โ€” create one below.").weak()); } let folders = self.folders.clone(); + // Precompute each folder's descendant set so the parent + // picker can exclude choices that would create a cycle. + let descendants = |fid: i64| -> std::collections::HashSet { + let mut out = std::collections::HashSet::new(); + let mut stack = vec![fid]; + while let Some(cur) = stack.pop() { + for f in &folders { + if f.parent_id == Some(cur) && out.insert(f.id) { + stack.push(f.id); + } + } + } + out + }; for f in &folders { let member_count = self.library.iter() .filter(|c| c.folder_id == Some(f.id)) @@ -1991,6 +2062,32 @@ impl App { } }); }); + // Parent picker for nesting. Banned = self + descendants. + let banned = { + let mut b = descendants(f.id); + b.insert(f.id); + b + }; + let current_parent_name = f.parent_id + .and_then(|pid| folders.iter().find(|o| o.id == pid)) + .map(|o| o.name.as_str()) + .unwrap_or("โ€” top level โ€”"); + ui.horizontal(|ui| { + ui.label(egui::RichText::new(" โ†ณ parent:").small().weak()); + egui::ComboBox::from_id_salt(("folder_parent", f.id)) + .selected_text(current_parent_name) + .show_ui(ui, |ui| { + if ui.selectable_label(f.parent_id.is_none(), "โ€” top level โ€”").clicked() { + to_reparent = Some((f.id, None)); + } + for o in &folders { + if banned.contains(&o.id) { continue; } + if ui.selectable_label(f.parent_id == Some(o.id), &o.name).clicked() { + to_reparent = Some((f.id, Some(o.id))); + } + } + }); + }); ui.separator(); } ui.add_space(6.0); @@ -2015,6 +2112,15 @@ impl App { } } } + if let Some((id, new_parent)) = to_reparent { + match self.db.set_folder_parent(id, new_parent) { + Ok(()) => { + self.folders = self.db.list_folders().unwrap_or_default(); + self.status = "Folder moved".to_string(); + } + Err(e) => self.status = format!("Move folder: {e}"), + } + } if let Some((id, old_name)) = to_rename { // Inline rename: stash the id in a small confirm-style flow. // For brevity in v1 the rename is handled via a system prompt-free diff --git a/src/database.rs b/src/database.rs index ef6ccf9..e203029 100644 --- a/src/database.rs +++ b/src/database.rs @@ -30,6 +30,8 @@ pub struct FolderRecord { pub id: i64, pub name: String, pub position: i64, + /// Parent folder id for N-level nesting. `None` = top-level folder. + pub parent_id: Option, } /// In-memory representation of the `video_flags` table. Each set holds the @@ -185,6 +187,19 @@ impl Database { PRIMARY KEY (target_kind, target_id) );", )?; + + // โ”€โ”€ Migration: folders.parent_id (N-level nesting) โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + // `folders` predates nesting. Add the column idempotently โ€” SQLite + // has no `ADD COLUMN IF NOT EXISTS`, so we attempt the ALTER and + // swallow the "duplicate column name" error that fires on an + // already-migrated DB. NULL parent_id = top-level folder. + let conn = self.conn(); + match conn.execute("ALTER TABLE folders ADD COLUMN parent_id INTEGER REFERENCES folders(id) ON DELETE CASCADE", []) { + Ok(_) => {} + Err(rusqlite::Error::SqliteFailure(_, Some(msg))) + if msg.contains("duplicate column") => {} + Err(e) => return Err(e), + } Ok(()) } @@ -276,17 +291,65 @@ impl Database { /// has a deterministic order even before drag-to-reorder ships. pub fn list_folders(&self) -> Result> { let conn = self.conn(); - let mut stmt = conn.prepare("SELECT id, name, position FROM folders ORDER BY position, id")?; + let mut stmt = conn.prepare("SELECT id, name, position, parent_id FROM folders ORDER BY position, id")?; let rows = stmt.query_map([], |row| { Ok(FolderRecord { id: row.get(0)?, name: row.get(1)?, position: row.get(2)?, + parent_id: row.get(3)?, }) })?; Ok(rows.filter_map(std::result::Result::ok).collect()) } + /// Reparent a folder. `new_parent = None` makes it top-level. + /// + /// Refuses to create a cycle: a folder can't become its own ancestor. + /// We walk up from the proposed new parent; if we hit `id` along the + /// way the move would form a loop and we return a friendly error + /// instead of corrupting the tree. + pub fn set_folder_parent(&self, id: i64, new_parent: Option) -> Result<()> { + let conn = self.conn(); + if let Some(parent) = new_parent { + if parent == id { + return Err(rusqlite::Error::SqliteFailure( + rusqlite::ffi::Error::new(rusqlite::ffi::SQLITE_CONSTRAINT), + Some("a folder can't be its own parent".into()), + )); + } + // Walk ancestors of `parent`; if `id` appears, this would cycle. + let mut cur = Some(parent); + let mut guard = 0; + while let Some(c) = cur { + if c == id { + return Err(rusqlite::Error::SqliteFailure( + rusqlite::ffi::Error::new(rusqlite::ffi::SQLITE_CONSTRAINT), + Some("that move would nest a folder inside its own descendant".into()), + )); + } + // Defensive bound in case the table is already corrupt. + guard += 1; + if guard > 10_000 { break; } + cur = match conn.query_row( + "SELECT parent_id FROM folders WHERE id = ?1", + [c], + |r| r.get::<_, Option>(0), + ) { + Ok(p) => p, + // Row gone (shouldn't happen mid-walk) โ†’ stop walking. + Err(rusqlite::Error::QueryReturnedNoRows) => None, + Err(e) => return Err(e), + }; + } + } + conn.execute( + "UPDATE folders SET parent_id = ?1 WHERE id = ?2", + rusqlite::params![new_parent, id], + )?; + Ok(()) + } + /// Set or clear a channel's folder assignment. `folder_id = None` /// deletes the row so the channel reverts to "Unfiled". pub fn set_channel_folder( @@ -986,6 +1049,36 @@ mod restore_tests { assert_eq!(db.get_all_notes().unwrap().len(), 1); } + #[test] + fn folder_nesting_and_cycle_guard() { + let dir = ScratchDir::new("folder-nest"); + let db = Database::open(&dir.join("nest.db")).unwrap(); + + let a = db.create_folder("A").unwrap(); + let b = db.create_folder("B").unwrap(); + let c = db.create_folder("C").unwrap(); + + // Nest B under A, C under B. + db.set_folder_parent(b, Some(a)).unwrap(); + db.set_folder_parent(c, Some(b)).unwrap(); + + let folders = db.list_folders().unwrap(); + let by_id = |id: i64| folders.iter().find(|f| f.id == id).unwrap(); + assert_eq!(by_id(a).parent_id, None); + assert_eq!(by_id(b).parent_id, Some(a)); + assert_eq!(by_id(c).parent_id, Some(b)); + + // A folder can't be its own parent. + assert!(db.set_folder_parent(a, Some(a)).is_err()); + + // Can't nest A under C (C is A's descendant โ†’ cycle). + assert!(db.set_folder_parent(a, Some(c)).is_err()); + + // Moving back to top level works. + db.set_folder_parent(c, None).unwrap(); + assert_eq!(db.list_folders().unwrap().iter().find(|f| f.id == c).unwrap().parent_id, None); + } + #[test] fn notes_merge_keeps_later_on_restore() { let dir = ScratchDir::new("notes-merge"); diff --git a/src/web.rs b/src/web.rs index f593000..8e0bab1 100644 --- a/src/web.rs +++ b/src/web.rs @@ -1766,6 +1766,11 @@ struct FolderCreateBody { name: String } #[derive(Deserialize)] struct FolderRenameBody { name: String } +/// Body for `POST /api/folders/:id/parent`. `parent_id = null` moves the +/// folder to the top level. +#[derive(Deserialize)] +struct FolderParentBody { parent_id: Option } + #[derive(Deserialize)] struct AssignFolderBody { folder_id: Option } @@ -1807,6 +1812,28 @@ async fn post_rename_folder( } } +/// `POST /api/folders/:id/parent` โ€” reparent a folder for nesting. +/// `parent_id = null` makes it top-level. The DB layer rejects cycles +/// (a folder can't become its own descendant) with a 400. +async fn post_folder_parent( + State(state): State>, + Path(id): Path, + Json(body): Json, +) -> impl IntoResponse { + match state.db.set_folder_parent(id, body.parent_id) { + Ok(()) => { + bump_library_version(&state); + (StatusCode::OK, "ok").into_response() + } + // A constraint failure here is the cycle guard โ€” surface it as a + // 400 with the friendly message the DB layer attached. + Err(rusqlite::Error::SqliteFailure(_, Some(msg))) => { + (StatusCode::BAD_REQUEST, msg).into_response() + } + Err(e) => (StatusCode::INTERNAL_SERVER_ERROR, format!("db: {e}")).into_response(), + } +} + /// `DELETE /api/folders/:id` โ€” drop the folder. Member channels revert /// to "Unfiled" via the foreign-key cascade. async fn delete_folder( @@ -2403,6 +2430,7 @@ async fn serve(config: Config, shutdown_rx: std::sync::mpsc::Receiver<()>) { .route("/api/notes/:kind/:id", post(post_note)) .route("/api/folders", post(post_create_folder)) .route("/api/folders/:id/rename", post(post_rename_folder)) + .route("/api/folders/:id/parent", post(post_folder_parent)) .route("/api/folders/:id/check", post(post_check_folder)) .route("/api/backup/db", get(get_backup_db)) .route("/api/restore/db", post(post_restore_db)) diff --git a/src/web_ui/index.html b/src/web_ui/index.html index 1968b74..0a0fd40 100644 --- a/src/web_ui/index.html +++ b/src/web_ui/index.html @@ -318,11 +318,14 @@ function renderSidebar(){ // Render a single channel entry (collapsed or expanded with sub-actions). // Returned HTML is appended at the right point โ€” used both in the // folder loop and in the per-platform Unfiled loop below. - const renderChannel=(i)=>{ + // `pad` is the left padding in px for nesting under a folder; defaults + // to the flat 10px used by top-level (Unfiled / platform) channels. + const renderChannel=(i,pad)=>{ const ch=library[i]; const active=activeChannelIdx===i&&activePlaylistIdx===null&&!showContinue; const size=ch.size_bytes>0?' ยท '+fmtSize(ch.size_bytes):''; - let s=`
${esc(ch.platform_icon||'')} ${esc(ch.name)} (${ch.total_videos}${size})
`; + const padStyle=pad!=null?`padding-left:${pad}px;`:''; + let s=`
${esc(ch.platform_icon||'')} ${esc(ch.name)} (${ch.total_videos}${size})
`; if(activeChannelIdx===i&&!showContinue){ for(let pi=0;pi + ๐Ÿ“ Folders + + `; if(folders.length){ - h+=`
- ๐Ÿ“ Folders - -
`; - for(const f of folders){ - const members=[]; - for(let i=0;i${esc(f.name)} (${count})`; - for(const i of members)h+=renderChannel(i); - } - } else { - h+=`
- ๐Ÿ“ Folders - -
`; + // Recursively render the folder tree. parent_id===null (or a missing + // parent) marks a top-level folder; children indent one step further. + // A `seen` set guards against a malformed cycle in the data turning + // this into infinite recursion. + const childrenOf=(pid)=>folders.filter(f=>(f.parent_id??null)===pid); + // Count members including all descendant folders, so a parent folder's + // tally reflects everything nested under it. + const directMembers=(fid)=>{ + const out=[]; + for(let i=0;i{ + if(seen.has(fid))return 0; seen.add(fid); + let n=directMembers(fid).length; + for(const c of childrenOf(fid))n+=subtreeCount(c.id,seen); + return n; + }; + const renderFolderTree=(f,depth,seen)=>{ + if(seen.has(f.id))return; seen.add(f.id); + const pad=8+depth*12; + const total=subtreeCount(f.id,new Set()); + h+=`
${'โ”” '.repeat(0)}${esc(f.name)} (${total})
`; + // Direct member channels of this folder, indented under it. + for(const i of directMembers(f.id))h+=renderChannel(i,pad+6); + // Then nested child folders. + for(const c of childrenOf(f.id))renderFolderTree(c,depth+1,seen); + }; + const seen=new Set(); + for(const f of childrenOf(null))renderFolderTree(f,0,seen); + // Safety net: render any orphaned folder whose parent_id points at a + // now-deleted row (shouldn't happen with the FK cascade, but cheap). + for(const f of folders)if(!seen.has(f.id))renderFolderTree(f,0,seen); } // Channels not assigned to a folder still get platform grouping. @@ -963,10 +985,25 @@ async function clearChannelOptions(platform,handle,btn){ async function openFolderManager(){ const bg=document.createElement('div');bg.className='modal-bg';bg.onclick=e=>{if(e.target===bg)bg.remove()}; const folders=librarySnapshotFolders||[]; + // Build the set of descendant ids for each folder so the parent + // dropdown can grey out choices that would create a cycle (you can't + // nest a folder inside itself or any of its own descendants). + const childrenOf=(pid)=>folders.filter(f=>(f.parent_id??null)===pid); + const descendantsOf=(fid)=>{ + const out=new Set(); const stack=[fid]; + while(stack.length){const cur=stack.pop(); + for(const c of childrenOf(cur)){if(!out.has(c.id)){out.add(c.id);stack.push(c.id);}}} + return out; + }; const rows=folders.map(f=>{ const memberCount=library.filter(c=>c.folder_id===f.id).length; - return `
- ๐Ÿ“ ${esc(f.name)} (${memberCount} channel${memberCount===1?'':'s'}) + const banned=descendantsOf(f.id); banned.add(f.id); + const parentOpts=[``] + .concat(folders.filter(o=>!banned.has(o.id)).map(o=> + ``)).join(''); + return `
+ ๐Ÿ“ ${esc(f.name)} (${memberCount} channel${memberCount===1?'':'s'}) + ${memberCount>0?``:''} @@ -1009,6 +1046,18 @@ async function renameFolderPrompt(id,oldName){ openFolderManager(); }catch(e){setStatus('Error: '+e.message)} } +async function setFolderParent(id,parentVal){ + // Empty string from the dropdown means "top level" โ†’ null parent. + const parent_id=parentVal===''?null:parseInt(parentVal,10); + try{ + const r=await api(`/api/folders/${id}/parent`,{method:'POST',headers:{'Content-Type':'application/json'},body:JSON.stringify({parent_id})}); + if(!r.ok)throw new Error(await r.text()); + setStatus('Folder moved'); + document.querySelector('.modal-bg')?.remove(); + await loadLibrary(); + openFolderManager(); + }catch(e){setStatus('Error: '+e.message);await loadLibrary();} +} async function checkFolder(id,name,btn){ btn.disabled=true; try{