From 22478c3a56f6ff47e60d8dc79861a2efc99928bb Mon Sep 17 00:00:00 2001 From: Timothy Allen Date: Fri, 2 Oct 2026 23:42:12 +0200 Subject: [PATCH] Simplify the upload state and the story page data The pause of the user and a missing ingest folder are separate checks. One function gives the upload state to the settings and the story page. A route that returns only a story no longer loads its chapters or asks CWA for its book. The story page data loses the values that no page reads. --- bindery/orchestrator.py | 70 +++++++++++++++++++---------------- bindery/static/js/story.js | 8 ++-- bindery/web.py | 6 +-- tests/js/flows.test.mjs | 2 +- tests/js/upload-hold.test.mjs | 18 ++++----- tests/test_api_flows.py | 12 ++++++ tests/test_optional_cwa.py | 5 +-- 7 files changed, 70 insertions(+), 51 deletions(-) diff --git a/bindery/orchestrator.py b/bindery/orchestrator.py index 1a9257f..1e5953e 100644 --- a/bindery/orchestrator.py +++ b/bindery/orchestrator.py @@ -62,10 +62,6 @@ def _link_books() -> bool: return _linking_available() and store.get_setting(None, "link_books") is not False -def _can_match_books() -> bool: - return _link_books() - - def default_locked_fields() -> list[str]: return [f for f in config.DEFAULT_LOCKED_FIELDS if f in METADATA_FIELDS] @@ -202,7 +198,7 @@ def list_stories() -> list[dict[str, Any]]: stories = store.list_stories() held = store.story_settings("upload_held") unapproved = store.story_settings("unapproved_link") - show_holds = _link_books() and store.get_setting(None, "hold_new_links") and not _uploads_paused() + show_holds = _link_books() and store.get_setting(None, "hold_new_links") and _can_upload() for s in stories: s["busy"] = job_queue.has_active(s["id"]) s["upload_held"] = bool(held.get(s["id"])) @@ -229,35 +225,35 @@ def story_cover(story_id: int) -> dict[str, Any] | None: return cover if cover and cover["mime"] in COVER_TYPES else None -def story_details(story_id: int) -> dict[str, Any]: +def story_view(story_id: int) -> dict[str, Any]: + """A story as the story page shows it, without its chapters and its library book.""" story = _require_story(story_id) story.pop("metahtml", None) cover = story_cover(story_id) # The version changes with the image, so a browser can keep a cover until the site changes it. story["cover"] = hashlib.sha256(cover["data"]).hexdigest()[:16] if cover else None - book, library_error = None, None - if story["calibre_uuid"] and _can_match_books(): - try: - book = cwa().get_book(story["calibre_uuid"], timeout=PAGE_TIMEOUT) - except CwaError as e: - library_error = str(e) story["recheck_days"] = store.get_setting(story_id, "recheck_days") story["last_full_recheck"] = store.get_setting(story_id, "last_full_recheck") story["upload_held"] = bool(store.get_setting(story_id, "upload_held")) story["needs_approval"] = _needs_approval(story_id) + return story + + +def story_details(story_id: int) -> dict[str, Any]: + story = story_view(story_id) + book, library_error = None, None + if story["calibre_uuid"] and _link_books(): + try: + book = cwa().get_book(story["calibre_uuid"], timeout=PAGE_TIMEOUT) + except CwaError as e: + library_error = str(e) return { "recent_chapters": _setting_recent_chapters(), - "uploads_paused": _uploads_paused(), - "ingest_available": config.INGEST_DIR is not None, - "link_books": _link_books(), - "library_matching_available": _can_match_books(), + **_upload_state(), "story": story, "chapters": store.list_chapters(story_id), - "rules": store.rules(story_id), "book": book, - "library_available": library_available(), "library_error": library_error, - "jobs": job_queue.get_status(story_id), } @@ -335,7 +331,7 @@ def edit_metadata(story_id: int, values: dict[str, Any]) -> dict[str, Any]: queued = None if story["metahtml"] and (story["calibre_id"] or story["ingested_at"]): queued = job_queue.add(Job(mode=DownloadMode.REBUILD, story_id=story_id, title=title)).to_dict() - return {"story": story_details(story_id)["story"], "job": queued} + return {"story": story_view(story_id), "job": queued} # Defaults for the job list settings. Each pair is how many hours of finished jobs the Jobs @@ -360,11 +356,9 @@ def get_settings() -> dict[str, Any]: ini = config.PERSONAL_INI return { "recent_chapters": _setting_recent_chapters(), - "link_books": _link_books(), + **_upload_state(), "linking_available": _linking_available(), "hold_new_links": bool(store.get_setting(None, "hold_new_links")), - "pause_uploads": _uploads_paused(), - "ingest_available": config.INGEST_DIR is not None, "personal_ini": ini.read_text(encoding="utf-8") if ini.exists() else "", **job_list_settings(), } @@ -433,7 +427,7 @@ def save_settings( partial = config.PERSONAL_INI.with_name("personal.ini.part") partial.write_text(personal_ini, encoding="utf-8") partial.replace(config.PERSONAL_INI) - if released and not _uploads_paused(): + if released and _can_upload(): _release_held_uploads() return {**get_settings(), "warnings": warnings} @@ -601,7 +595,7 @@ def approve_upload(story_id: int, book_uuid: str | None) -> dict[str, Any] | Non if (book_uuid or None) != story["calibre_uuid"]: raise ValueError("The library link of this story changed. Reload the page, and check the link again.") store.set_setting(story_id, "unapproved_link", None) - if not _uploads_paused() and store.get_setting(story_id, "upload_held") and story["metahtml"]: + if _can_upload() and store.get_setting(story_id, "upload_held") and story["metahtml"]: return job_queue.add(Job(mode=DownloadMode.REBUILD, story_id=story_id, title=story["title"])).to_dict() return None @@ -787,7 +781,7 @@ def _link(story: dict[str, Any], book_id: int, book_uuid: str) -> None: def _linked_book(story: dict[str, Any]) -> dict[str, Any] | None: """The library book for a story. Links the story if CWA has a book with its URL.""" - if not _can_match_books(): + if not _link_books(): if story["calibre_id"] is not None: return {"id": story["calibre_id"], "uuid": story["calibre_uuid"], "title": story["title"]} return None @@ -834,8 +828,22 @@ NOT_FOUND = "Cannot find this story in CWA. Link to library book, or send as new # with the next upload or an approval. "upload_held" marks a built book that did not go to CWA. -def _uploads_paused() -> bool: - return config.INGEST_DIR is None or bool(store.get_setting(None, "pause_uploads")) +def _paused() -> bool: + """Whether the user paused the uploads.""" + return bool(store.get_setting(None, "pause_uploads")) + + +def _can_upload() -> bool: + return config.INGEST_DIR is not None and not _paused() + + +def _upload_state() -> dict[str, bool]: + """What the pages show about uploads. Without an ingest folder, the uploads count as paused.""" + return { + "link_books": _link_books(), + "pause_uploads": not _can_upload(), + "ingest_available": config.INGEST_DIR is not None, + } def _link_changed(story_id: int) -> None: @@ -859,9 +867,9 @@ def _release_held_uploads() -> None: def _hold_reason(story: dict[str, Any], book: dict[str, Any] | None) -> str | None: - if _uploads_paused(): - if config.INGEST_DIR is None: - return "Not sent. INGEST_DIR is not set." + if config.INGEST_DIR is None: + return "Not sent. INGEST_DIR is not set." + if _paused(): return "Not sent. Turn off pause to send." if not _needs_approval(story["id"]): return None diff --git a/bindery/static/js/story.js b/bindery/static/js/story.js index 736b79c..30ceed7 100644 --- a/bindery/static/js/story.js +++ b/bindery/static/js/story.js @@ -174,7 +174,7 @@ export function storyPage(main, { storyId }, config) { }; const approveAsNew = async () => { - const action = details.uploads_paused + const action = details.pause_uploads ? 'Unlink the library book, and approve this story as a new book? Uploads remain paused. ' : 'Unlink the library book, and send this story to CWA as a new book? '; const message = action + @@ -193,7 +193,7 @@ export function storyPage(main, { storyId }, config) { const unlinked = !linking && story.calibre_id == null; // A previously uploaded book needs a known target before approval. const approval = linking && story.needs_approval && (story.calibre_id != null || !story.ingested_at); - const paused = details.uploads_paused && (story.upload_held || approval); + const paused = details.pause_uploads && (story.upload_held || approval); if (!paused && !approval && !unlinked) return null; let target = 'Approve this story as a new CWA book.'; if (book) { @@ -223,11 +223,11 @@ export function storyPage(main, { storyId }, config) { h( 'div', { class: 'controls' }, - button(story.upload_held && !details.uploads_paused ? 'Approve and send' : 'Approve', () => approve(story.calibre_uuid), { + button(story.upload_held && !details.pause_uploads ? 'Approve and send' : 'Approve', () => approve(story.calibre_uuid), { class: 'primary', }), story.calibre_uuid && button( - details.uploads_paused ? 'Approve as a new book instead' : 'Send as a new book instead', approveAsNew, + details.pause_uploads ? 'Approve as a new book instead' : 'Send as a new book instead', approveAsNew, ), ), ], diff --git a/bindery/web.py b/bindery/web.py index 8db50ca..d6c5671 100644 --- a/bindery/web.py +++ b/bindery/web.py @@ -368,7 +368,7 @@ def story_settings(story_id: int) -> Any: data.get("auto_update"), None if days is None else _int(days, "recheck_days"), ) - return orchestrator.story_details(story_id)["story"] + return orchestrator.story_view(story_id) @app.put("/api/stories//metadata") @@ -382,13 +382,13 @@ def edit_metadata(story_id: int) -> Any: @app.post("/api/stories//link") def link(story_id: int) -> Any: orchestrator.link_story(story_id, _json().get("uuid") or None) - return orchestrator.story_details(story_id)["story"] + return orchestrator.story_view(story_id) @app.post("/api/stories//approve") def approve_upload(story_id: int) -> Any: job = orchestrator.approve_upload(story_id, _json().get("uuid") or None) - return {"story": orchestrator.story_details(story_id)["story"], "job": job} + return {"story": orchestrator.story_view(story_id), "job": job} @app.post("/api/stories//chapters") diff --git a/tests/js/flows.test.mjs b/tests/js/flows.test.mjs index 74913f2..6b1bda0 100644 --- a/tests/js/flows.test.mjs +++ b/tests/js/flows.test.mjs @@ -647,7 +647,7 @@ test('with CWA off, the pages show nothing about CWA', async () => { assert.ok(section(main, 'Library link')); const off = { ...config, cwa_enabled: false, library_available: false }; // Values that show a notice with CWA on. - Object.assign(details, { uploads_paused: true, ingest_available: false, link_books: false }); + Object.assign(details, { pause_uploads: true, ingest_available: false, link_books: false }); details.story.upload_held = true; await mountStory(off); assert.ok(section(main, 'Metadata')); diff --git a/tests/js/upload-hold.test.mjs b/tests/js/upload-hold.test.mjs index a3d42af..761f0f6 100644 --- a/tests/js/upload-hold.test.mjs +++ b/tests/js/upload-hold.test.mjs @@ -15,7 +15,7 @@ const details = (story, extra = {}) => ({ calibre_id: null, calibre_uuid: null, ingested_at: null, upload_held: false, needs_approval: false, ...story }, chapters: [], recent_chapters: 0, - uploads_paused: false, + pause_uploads: false, book: null, ...extra, }); @@ -192,7 +192,7 @@ for (const uuid of [null, 'book-7']) { test(`approval while paused saves the decision without promising an upload: ${uuid}`, async (t) => { const data = details( { calibre_id: uuid ? 7 : null, calibre_uuid: uuid, upload_held: true, needs_approval: true }, - { link_books: true, uploads_paused: true }, + { link_books: true, pause_uploads: true }, ); const { backend, notice } = await renderStory(t, data); assert.ok(find(notice, 'button', 'Approve')); @@ -208,7 +208,7 @@ for (const uuid of [null, 'book-7']) { test('approval as a new book while paused keeps the pause in the confirmation', async (t) => { const data = details( { calibre_id: 7, calibre_uuid: 'book-7', upload_held: true, needs_approval: true }, - { link_books: true, uploads_paused: true }, + { link_books: true, pause_uploads: true }, ); const { backend, notice } = await renderStory(t, data); const confirm = t.mock.method(window, 'confirm', () => true); @@ -239,7 +239,7 @@ test('a new story asks for approval to go to CWA as a new book', async (t) => { test('a paused upload points to the settings', async (t) => { const { notice } = await renderStory( t, - details({ calibre_id: 7, calibre_uuid: 'book-7', ingested_at: '2026-01-01', upload_held: true }, { book, uploads_paused: true }), + details({ calibre_id: 7, calibre_uuid: 'book-7', ingested_at: '2026-01-01', upload_held: true }, { book, pause_uploads: true }), ); assert.match(notice.textContent, /Uploads to CWA are paused/); assert.equal(find(notice, 'a', 'Settings').attributes.href, '#/settings'); @@ -253,7 +253,7 @@ for (const savedId of [null, 7]) { const { notice } = await renderStory(t, details( { calibre_id: savedId, calibre_uuid: savedId ? 'book-7' : null, upload_held: true, needs_approval: needsApproval }, - { link_books: false, uploads_paused: paused }, + { link_books: false, pause_uploads: paused }, )); if (savedId !== null && !paused) { assert.equal(notice, undefined); @@ -294,7 +294,7 @@ test('a story that went to CWA without a link shows no approval until Bindery fi test('missing INGEST_DIR points to the environment rather than the disabled pause switch', async (t) => { const { notice } = await renderStory(t, details( - { upload_held: true }, { uploads_paused: true, ingest_available: false }, + { upload_held: true }, { pause_uploads: true, ingest_available: false }, )); assert.equal(notice.textContent, 'Set INGEST_DIR and restart Bindery to enable uploads.'); assert.equal(nodes(notice).filter((el) => el.tagName === 'button').length, 0); @@ -303,7 +303,7 @@ test('missing INGEST_DIR points to the environment rather than the disabled paus test('missing library sources leave uploads enabled with the duplicate warning', async (t) => { const { notice } = await renderStory(t, details( { ingested_at: '2026-01-01', upload_held: true, needs_approval: true }, - { link_books: false, library_matching_available: false }, + { link_books: false }, )); assert.match(notice.textContent, /If CWA imports them, updates will create duplicate books/); assert.equal(nodes(notice).filter((el) => el.tagName === 'button').length, 0); @@ -312,7 +312,7 @@ test('missing library sources leave uploads enabled with the duplicate warning', test('a new book asks for approval when library linking is enabled', async (t) => { const { notice } = await renderStory(t, details( { upload_held: true, needs_approval: true }, - { link_books: true, library_matching_available: true }, + { link_books: true }, )); assert.match(notice.textContent, /^Approve this story as a new CWA book\./); assert.ok(find(notice, 'button', 'Approve and send')); @@ -320,7 +320,7 @@ test('a new book asks for approval when library linking is enabled', async (t) = test('approval before a build still shows the pause and never offers to send', async (t) => { const { notice } = await renderStory(t, details( - { needs_approval: true }, { uploads_paused: true }, + { needs_approval: true }, { pause_uploads: true }, )); assert.match(notice.textContent, /Uploads to CWA are paused\. Resume them in Settings\./); assert.ok(find(notice, 'button', 'Approve')); diff --git a/tests/test_api_flows.py b/tests/test_api_flows.py index bf11832..15800a3 100644 --- a/tests/test_api_flows.py +++ b/tests/test_api_flows.py @@ -187,3 +187,15 @@ def test_chapter_requests_without_ids_name_what_to_select(client, story): for body in ({}, {"chapter_ids": None}): response = client.post(f"/api/stories/{sid}/chapters/delete", json=body, headers=H) assert (response.status_code, response.json["error"]) == (400, "Select the chapters to delete") + + +def test_a_story_change_does_not_ask_cwa_for_the_book(client, story, monkeypatch): + sid, _ = story + o.store.update_story(sid, calibre_id=7, calibre_uuid="book-7") + monkeypatch.setattr(config, "CWA_URL", "https://cwa.example") + cwa = Mock() + monkeypatch.setattr(o, "cwa", lambda: cwa) + response = client.put(f"/api/stories/{sid}/settings", json={"recheck_days": 4}, headers=H) + assert response.json["recheck_days"] == 4 + assert "chapters" not in response.json + cwa.get_book.assert_not_called() diff --git a/tests/test_optional_cwa.py b/tests/test_optional_cwa.py index d5a8737..a70412f 100644 --- a/tests/test_optional_cwa.py +++ b/tests/test_optional_cwa.py @@ -218,7 +218,7 @@ def test_build_without_ingest_succeeds_and_keeps_the_book_local(monkeypatch, tmp assert list(config.TMP_DIR.glob(f"story-{story_id}-*.epub")) assert not list((tmp_path / "ingest").iterdir()) details = orchestrator.story_details(story_id) - assert details["uploads_paused"] is True + assert details["pause_uploads"] is True assert details["story"]["upload_held"] is True assert details["library_error"] is None @@ -454,12 +454,11 @@ def test_missing_library_connection_is_not_an_approval_hold(client, monkeypatch) assert details["story"]["upload_held"] is False assert details["story"]["needs_approval"] is False assert details["link_books"] is False - assert details["library_matching_available"] is False assert details["ingest_available"] is True monkeypatch.setattr(config, "INGEST_DIR", None) assert client.get(f"/api/stories/{story_id}").json["ingest_available"] is False monkeypatch.setattr(config, "CWA_URL", "https://cwa.invalid") - assert client.get(f"/api/stories/{story_id}").json["library_matching_available"] is True + assert client.get(f"/api/stories/{story_id}").json["link_books"] is True @pytest.mark.parametrize("missing_ingest", [False, True])