diff --git a/bindery/orchestrator.py b/bindery/orchestrator.py index 22510b2..f771ea2 100644 --- a/bindery/orchestrator.py +++ b/bindery/orchestrator.py @@ -482,6 +482,34 @@ def set_chapters_included(story_id: int, chapter_ids: list[int], included: bool) return queue_story(story_id, DownloadMode.REBUILD.value) +def move_chapters(story_id: int, chapter_ids: list[int], after_id: int | None) -> dict[str, Any]: + """Move chapters, in their current order, to directly after the chapter `after_id`, or to the start with None.""" + order = [c["id"] for c in store.list_chapters(story_id)] + moving = [cid for cid in order if cid in set(chapter_ids)] + if not moving: + raise ValueError("Select the chapters to move") + if after_id is not None and after_id not in order: + raise KeyError(f"No chapter {after_id} in this story") + if after_id in moving: + raise ValueError("The chapters cannot go after a chapter that moves with them") + rest = [cid for cid in order if cid not in moving] + at = rest.index(after_id) + 1 if after_id is not None else 0 + store.set_positions(story_id, rest[:at] + moving + rest[at:]) + return queue_story(story_id, DownloadMode.REBUILD.value) + + +def delete_chapters(story_id: int, chapter_ids: list[int]) -> dict[str, Any]: + """Delete chapters and their revisions. A chapter that the site still lists comes back with the next download.""" + order = [c["id"] for c in store.list_chapters(story_id)] + deleting = [cid for cid in order if cid in set(chapter_ids)] + if not deleting: + raise ValueError("Select the chapters to delete") + if len(deleting) == len(order): + raise ValueError("A story must keep at least one chapter. To remove the story, stop tracking it") + store.delete_chapters(story_id, deleting) + return queue_story(story_id, DownloadMode.REBUILD.value) + + def link_story(story_id: int, book_uuid: str | None) -> None: """Link a story to a library book by its uuid, or unlink it with None.""" story = _require_story(story_id) diff --git a/bindery/static/js/story.js b/bindery/static/js/story.js index 6c83854..6c3399d 100644 --- a/bindery/static/js/story.js +++ b/bindery/static/js/story.js @@ -87,7 +87,12 @@ export function storyPage(main, { storyId }, config) { const metadata = h('div', { class: 'metadata' }); const updates = h('div'); const chapters = h('div'); - const selectAll = h('span', { class: 'chapter-select-all' }); + // One dialog for Move… and Delete…. It names the selected chapters before anything changes. + const dialog = h('dialog', { class: 'chapter-dialog', 'aria-labelledby': 'chapter-dialog-title' }); + let moveAfterId = null; + // A shift-click on a chapter checkbox sets every chapter from the last one clicked. + let anchor = null; + let shift = false; const jobs = jobPanel({ hours: config.story_jobs_hours, count: config.story_jobs_count }); const linked = h('div'); const searchResults = h('div'); @@ -381,8 +386,17 @@ export function storyPage(main, { storyId }, config) { function renderChapters() { const list = details.chapters; + const ids = list.map((c) => c.id); + const pick = (i, on) => { + const from = shift ? ids.indexOf(anchor) : -1; + const range = from < 0 ? [ids[i]] : ids.slice(Math.min(from, i), Math.max(from, i) + 1); + for (const cid of range) on ? selected.add(cid) : selected.delete(cid); + anchor = ids[i]; + shift = false; + if (range.length > 1) renderChapters(); + }; let number = 0; - const rows = list.map((c) => { + const rows = list.map((c, i) => { if (c.included) number += 1; const route = `/story/${id}/chapter/${c.id}`; return h( @@ -391,8 +405,10 @@ export function storyPage(main, { storyId }, config) { h( 'td', {}, - checkbox(selected.has(c.id), (on) => (on ? selected.add(c.id) : selected.delete(c.id)), { + checkbox(selected.has(c.id), (on) => pick(i, on), { 'aria-label': `Select ${chapterTitle(c.title)}`, + title: 'Shift-click to select all chapters from the last one you clicked', + onclick: (e) => (shift = e.shiftKey), }), ), h('td', { 'data-label': '#' }, c.included ? String(number) : ''), @@ -412,11 +428,12 @@ export function storyPage(main, { storyId }, config) { selected = new Set(on ? list.map((c) => c.id) : []); renderChapters(); }, { 'aria-label': 'Select all chapters' }); - fill(selectAll, h('label', {}, all, ' Select all')); + // A phone hides the other headings, so the text shows only there. + const selectAll = h('label', {}, all, h('span', { class: 'select-all-text' }, ' Select all')); fill( chapters, table( - ['', '#', 'Title', 'State', 'Revisions', 'Added'], + [selectAll, '#', 'Title', 'State', 'Revisions', 'Added'], rows.length ? rows : h('tr', {}, h('td', { colspan: 6, class: 'empty' }, 'No chapters stored yet.')), 'chapters-table', ), @@ -462,6 +479,64 @@ export function storyPage(main, { storyId }, config) { await load(); }; + const finish = async (message) => { + dialog.close(); + toast(message); + refreshJobs(); + await load(); + }; + + const moveForm = (picked, chapterIds) => { + const others = details.chapters.filter((c) => !selected.has(c.id)); + const after = h( + 'select', + { id: 'move-after' }, + h('option', { value: '' }, 'The start'), + others.map((c) => h('option', { value: String(c.id) }, chapterTitle(c.title))), + ); + after.value = others.some((c) => c.id === moveAfterId) ? String(moveAfterId) : ''; + return [ + h('div', { class: 'controls' }, h('label', { for: 'move-after' }, picked.length === 1 ? 'Put it after' : 'Put them after'), after), + button('Move', async () => { + moveAfterId = after.value === '' ? null : Number(after.value); + await api(`stories/${id}/chapters/move`, 'POST', { chapter_ids: chapterIds, after: moveAfterId }); + await finish('Chapters moved. The book is being rebuilt.'); + }, { class: 'primary' }), + ]; + }; + + const deleteForm = (picked, chapterIds) => [ + h( + 'p', + {}, + `Bindery deletes ${picked.length === 1 ? 'this chapter and its' : 'these chapters and their'} revisions. `, + 'You cannot undo this.', + picked.some((c) => c.on_site) ? ' A chapter that the site still lists comes back with the next download of new chapters.' : '', + ), + button('Delete', async () => { + await api(`stories/${id}/chapters/delete`, 'POST', { chapter_ids: chapterIds }); + await finish('Chapters deleted. The book is being rebuilt.'); + }, { class: 'danger' }), + ]; + + const openDialog = (verb) => { + if (!selected.size) { + toast('Select chapters first', true); + return; + } + const picked = details.chapters.filter((c) => selected.has(c.id)); + const chapterIds = picked.map((c) => c.id); + const [body, action] = (verb === 'Move' ? moveForm : deleteForm)(picked, chapterIds); + fill( + dialog, + h('h3', { id: 'chapter-dialog-title' }, `${verb} ${plural(picked.length, 'chapter')}`), + h('ol', { class: 'dialog-chapters' }, picked.map((c) => h('li', {}, chapterTitle(c.title)))), + body, + h('div', { class: 'dialog-actions' }, h('button', { type: 'button', onclick: () => dialog.close() }, 'Cancel'), action), + ); + dialog.showModal(); + }; + const modeHint = h('p', { class: 'hint' }, MODE_HINTS[mode]); const modes = SITE_MODES.map((m) => h( @@ -601,7 +676,6 @@ export function storyPage(main, { storyId }, config) { 'details', { class: 'card', open: true }, h('summary', {}, h('h2', {}, 'Chapters')), - selectAll, h( 'div', { class: 'controls chapter-actions' }, @@ -610,8 +684,11 @@ export function storyPage(main, { storyId }, config) { ), button('Take out of book', () => setIncluded(false)), button('Put back in book', () => setIncluded(true)), + button('Move…', () => openDialog('Move')), + button('Delete…', () => openDialog('Delete'), { class: 'danger' }), ), chapters, + dialog, ), epubImport(id, 'Import chapters from an ePub'), h('details', { class: 'card', open: true }, h('summary', {}, h('h2', {}, 'Recent jobs')), jobs.el), diff --git a/bindery/static/style.css b/bindery/static/style.css index ce25c47..33b4238 100644 --- a/bindery/static/style.css +++ b/bindery/static/style.css @@ -390,9 +390,8 @@ details.card[open] > summary .summary-action { justify-content: space-between; } -.chapter-select-all { - display: block; - margin-top: 8px; +.select-all-text { + display: none; } /* Stays visible while the chapter list scrolls, so the buttons need no scrolling back up to @@ -768,8 +767,7 @@ textarea { display: block; } - .stories-table thead, - .chapters-table thead { + .stories-table thead { position: absolute; width: 1px; height: 1px; @@ -823,6 +821,57 @@ textarea { flex-basis: 100%; font-weight: 600; } + + /* Of the chapter list headings, only the Select all checkbox shows. */ + /* The left padding is the 1px border and 12px padding of a row card, so the checkboxes line up. */ + .chapters-table th { + border-bottom: 0; + padding: 0 0 8px 13px; + } + + .chapters-table th:not(:first-child) { + display: none; + } + + .select-all-text { + display: inline; + } +} + +/* The Move… and Delete… dialog of the chapter list. */ +.chapter-dialog { + width: min(32rem, calc(100vw - 32px)); + max-height: calc(100dvh - 32px); + padding: 16px 20px; + border: 1px solid var(--border); + border-radius: 8px; + background: var(--surface); + color: var(--text); +} + +.chapter-dialog::backdrop { + background: rgb(0 0 0 / 0.4); +} + +.chapter-dialog h3 { + margin: 0 0 8px; +} + +.dialog-chapters { + max-height: 40dvh; + overflow-y: auto; + margin: 0 0 12px; + padding: 8px 8px 8px 32px; + border: 1px solid var(--border); + border-radius: 6px; + background: var(--card-bg); +} + +.dialog-actions { + display: flex; + justify-content: flex-end; + gap: 8px; + margin-top: 16px; } /* Jobs */ diff --git a/bindery/store.py b/bindery/store.py index 587b3c7..5663834 100644 --- a/bindery/store.py +++ b/bindery/store.py @@ -345,6 +345,13 @@ class Store: [(i, cid, story_id) for i, cid in enumerate(chapter_ids)], ) + def delete_chapters(self, story_id: int, chapter_ids: list[int]) -> None: + """Delete chapters of the story and their revisions.""" + with self._connect() as conn: + conn.executemany( + "DELETE FROM chapters WHERE id = ? AND story_id = ?", [(cid, story_id) for cid in chapter_ids] + ) + def list_revisions(self, chapter_id: int) -> list[dict[str, Any]]: with self._connect() as conn: rows = conn.execute( diff --git a/bindery/web.py b/bindery/web.py index 7b41f2a..0c25526 100644 --- a/bindery/web.py +++ b/bindery/web.py @@ -332,6 +332,20 @@ def chapters_included(story_id: int) -> Any: ) +@app.post("/api/stories//chapters/move") +def move_chapters(story_id: int) -> Any: + data = _json() + after = data.get("after") + return orchestrator.move_chapters( + story_id, [int(c) for c in data.get("chapter_ids", [])], None if after is None else int(after) + ) + + +@app.post("/api/stories//chapters/delete") +def delete_chapters(story_id: int) -> Any: + return orchestrator.delete_chapters(story_id, [int(c) for c in _json().get("chapter_ids", [])]) + + @app.get("/api/stories//epub") def story_epub(story_id: int) -> Any: path, title = orchestrator.build_for_download(story_id) diff --git a/tests/js/flow-helpers.mjs b/tests/js/flow-helpers.mjs index 00fd0c8..a53a4cb 100644 --- a/tests/js/flow-helpers.mjs +++ b/tests/js/flow-helpers.mjs @@ -30,6 +30,8 @@ export class Element { removeAttribute(name) { delete this.attributes[name]; } addEventListener(name, fn) { this.listeners[name] = fn; } focus() { document.activeElement = this; } + showModal() { this.open = true; } + close() { this.open = false; } remove() { if (this.parent) this.parent.children = this.parent.children.filter((child) => child !== this); } get firstElementChild() { return this.children.find((child) => child instanceof Element); } get textContent() { return this.children.map((child) => child instanceof Element ? child.textContent : String(child)).join(''); } diff --git a/tests/js/flows.test.mjs b/tests/js/flows.test.mjs index 5b57d54..de93d44 100644 --- a/tests/js/flows.test.mjs +++ b/tests/js/flows.test.mjs @@ -275,13 +275,13 @@ test('the chapter table stacks on a narrow screen instead of scrolling sideways' await mountStory(); const wrap = nodes(main).find((el) => el.className === 'table-wrap chapters-table'); assert.ok(wrap); - const headings = nodes(wrap).filter((el) => el.tagName === 'th').map((el) => el.textContent); - assert.deepEqual(headings, ['', '#', 'Title', 'State', 'Revisions', 'Added']); + const headings = nodes(wrap).filter((el) => el.tagName === 'th'); + assert.deepEqual(headings.map((el) => el.textContent.trim()), ['Select all', '#', 'Title', 'State', 'Revisions', 'Added']); const dataLabels = nodes(wrap).filter((el) => el.tagName === 'td').map((el) => el.attributes['data-label']); const perRow = [undefined, '#', undefined, 'State', 'Revisions', 'Added']; assert.deepEqual(dataLabels, [...perRow, ...perRow]); - // "Select all" moved above the table, so a hidden table header on mobile does not hide it. - assert.ok(nodes(main).find((el) => el.tagName === 'label' && el.textContent.trim() === 'Select all')); + // The Select all checkbox is in the first heading. On a phone, it is the only heading that shows. + assert.ok(nodes(headings[0]).includes(field(main, 'Select all chapters'))); }); test('story list stacks on a narrow screen instead of scrolling sideways', async () => { @@ -348,6 +348,47 @@ test('chapter inclusion changes reload the story after queuing the rebuild', asy assert.equal(api.sent('GET', 'stories/1').length, 3); }); +test('shift-click selects a range of chapters, and Move sends them with the chapter to go after', async () => { + details.chapters.push({ id: 4, title: 'Three', url: 'chapter-3', included: true, on_site: true, current_rev: 1, revisions: 1 }); + await mountStory(); + api.on('POST', 'stories/1/chapters/move', {}); + const dialog = nodes(main).find((el) => el.tagName === 'dialog'); + const texts = (tag) => nodes(dialog).filter((el) => el.tagName === tag).map((el) => el.textContent); + await click(main, 'Move…'); + assert.ok(!dialog.open); + const box = (title) => field(main, `Select ${title}`); + const shiftClick = (el, on) => { + el.listeners.click({ shiftKey: true, target: el }); + return change(el, on); + }; + change(box('One'), true); + shiftClick(box('Three'), true); + assert.equal(box('Two').checked, true); + // The dialog names the chapters, and they are not in the list of chapters to go after. + await click(main, 'Move…'); + assert.ok(dialog.open); + assert.deepEqual(texts('h3'), ['Move 3 chapters']); + assert.deepEqual(texts('li'), ['One', 'Two', 'Three']); + assert.deepEqual(texts('option'), ['The start']); + await click(dialog, 'Cancel'); + assert.ok(!dialog.open); + assert.equal(writes().length, 0); + await click(main, 'Move…'); + await click(dialog, 'Move'); + assert.deepEqual(api.sent('POST', 'stories/1/chapters/move').at(-1).body, { chapter_ids: [2, 3, 4], after: null }); + assert.ok(!dialog.open); + shiftClick(box('One'), false); + change(box('Two'), true); + await click(main, 'Move…'); + assert.deepEqual(texts('option'), ['The start', 'One', 'Three']); + field(dialog, 'move-after').value = '4'; + await click(dialog, 'Move'); + assert.deepEqual(api.sent('POST', 'stories/1/chapters/move').at(-1).body, { chapter_ids: [3], after: 4 }); + // The next dialog starts with the last choice. + await click(main, 'Move…'); + assert.equal(field(dialog, 'move-after').value, '4'); +}); + test('unsaved metadata and update settings survive job completion, then save typed values', async () => { await mountStory(); startPolling(); @@ -449,6 +490,30 @@ test('library search trims the query and links by UUID', async () => { assert.equal(api.sent('GET', 'stories/1').length, 2); }); +test('deleting chapters asks in a dialog and warns about chapters that are on the site', async () => { + await mountStory(); + api.on('POST', 'stories/1/chapters/delete', {}); + const dialog = nodes(main).find((el) => el.tagName === 'dialog'); + const text = (tag) => nodes(dialog).find((el) => el.tagName === tag).textContent; + await click(main, 'Delete…'); + assert.ok(!dialog.open); + change(field(main, 'Select Two'), true); + await click(main, 'Delete…'); + assert.ok(dialog.open); + assert.equal(text('h3'), 'Delete 1 chapter'); + assert.equal(text('p'), 'Bindery deletes this chapter and its revisions. You cannot undo this.'); + await click(dialog, 'Cancel'); + change(field(main, 'Select One'), true); + await click(main, 'Delete…'); + assert.equal(text('h3'), 'Delete 2 chapters'); + assert.match(text('p'), /^Bindery deletes these chapters and their revisions\..*comes back with the next download/); + assert.equal(writes().length, 0); + await click(dialog, 'Delete'); + assert.deepEqual(api.sent('POST', 'stories/1/chapters/delete').at(-1).body, { chapter_ids: [2, 3] }); + assert.ok(!dialog.open); + assert.equal(api.sent('GET', 'stories/1').length, 2); +}); + test('sending as a new book and stopping tracking require confirmation', async () => { details.story.ingested_at = '2026-01-01'; await mountStory(); diff --git a/tests/test_api_flows.py b/tests/test_api_flows.py index 1ee55d6..a86cf35 100644 --- a/tests/test_api_flows.py +++ b/tests/test_api_flows.py @@ -64,6 +64,14 @@ def test_settings_and_inclusion_persist_and_delete_cascades(client, story): ) assert response.json["mode"] == "rebuild" assert not o.store.get_chapter(cid)["included"] + response = client.post(f"/api/stories/{sid}/chapters/move", json={"chapter_ids": [cid], "after": None}, headers=H) + assert response.json["mode"] == "rebuild" + move = client.post(f"/api/stories/{sid}/chapters/move", json={"chapter_ids": [cid], "after": 999}, headers=H) + assert move.status_code == 404 + # The story has only this chapter, so it cannot go. + response = client.post(f"/api/stories/{sid}/chapters/delete", json={"chapter_ids": [cid]}, headers=H) + assert response.status_code == 400 + assert "at least one chapter" in response.json["error"] o.store.set_rules(sid, [{"pattern": "a", "replacement": "b", "enabled": True}]) response = client.delete(f"/api/stories/{sid}", headers=H) assert response.json == {"deleted": sid} diff --git a/tests/test_worker_flows.py b/tests/test_worker_flows.py index ec1c66b..5d7ca9b 100644 --- a/tests/test_worker_flows.py +++ b/tests/test_worker_flows.py @@ -250,6 +250,47 @@ def test_inclusion_changes_only_the_requested_story_and_queues_a_rebuild(state): assert len(queue.get_status()) == 1 +def test_move_chapters_keeps_their_order_and_queues_a_rebuild(state): + store, queue = state + sid, other = add_story(store), add_story(store, "other") + ids = [store.add_chapter(sid, f"c{n}", str(n), "

x

", "h", "new", n) for n in range(5)] + foreign = store.add_chapter(other, "foreign", "Other", "

x

", "h", "new", 0) + + def titles(): + return [c["title"] for c in store.list_chapters(sid)] + + # The selection order does not count, and a chapter of another story is ignored. + assert o.move_chapters(sid, [ids[3], ids[1], foreign], ids[4])["mode"] == "rebuild" + assert titles() == ["0", "2", "4", "1", "3"] + o.move_chapters(sid, [ids[4], ids[3]], None) + assert titles() == ["4", "3", "0", "2", "1"] + assert len(queue.get_status()) == 1 + with pytest.raises(ValueError, match="Select"): + o.move_chapters(sid, [foreign], None) + with pytest.raises(ValueError, match="moves with them"): + o.move_chapters(sid, [ids[0], ids[1]], ids[1]) + with pytest.raises(KeyError): + o.move_chapters(sid, [ids[0]], foreign) + + +def test_delete_chapters_removes_their_revisions_and_keeps_one_chapter(state): + store, queue = state + sid, other = add_story(store), add_story(store, "other") + ids = [store.add_chapter(sid, f"c{n}", str(n), "

x

", "h", "new", n) for n in range(3)] + store.add_revision(ids[1], "

y

", "h2", "recheck") + foreign = store.add_chapter(other, "foreign", "Other", "

x

", "h", "new", 0) + + assert o.delete_chapters(sid, [ids[1], foreign])["mode"] == "rebuild" + assert [c["id"] for c in store.list_chapters(sid)] == [ids[0], ids[2]] + assert store.list_revisions(ids[1]) == [] + assert store.get_chapter(foreign) + with pytest.raises(ValueError, match="Select"): + o.delete_chapters(sid, [foreign]) + with pytest.raises(ValueError, match="at least one chapter"): + o.delete_chapters(sid, [ids[0], ids[2]]) + assert len(store.list_chapters(sid)) == 2 + + @pytest.mark.parametrize( "mode,chapters,fields,message", [