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.
This commit is contained in:
tim
2026-10-03 08:09:41 +02:00
parent 2b1a26d1da
commit 22478c3a56
7 changed files with 70 additions and 51 deletions
+39 -31
View File
@@ -62,10 +62,6 @@ def _link_books() -> bool:
return _linking_available() and store.get_setting(None, "link_books") is not False 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]: def default_locked_fields() -> list[str]:
return [f for f in config.DEFAULT_LOCKED_FIELDS if f in METADATA_FIELDS] 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() stories = store.list_stories()
held = store.story_settings("upload_held") held = store.story_settings("upload_held")
unapproved = store.story_settings("unapproved_link") 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: for s in stories:
s["busy"] = job_queue.has_active(s["id"]) s["busy"] = job_queue.has_active(s["id"])
s["upload_held"] = bool(held.get(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 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 = _require_story(story_id)
story.pop("metahtml", None) story.pop("metahtml", None)
cover = story_cover(story_id) cover = story_cover(story_id)
# The version changes with the image, so a browser can keep a cover until the site changes it. # 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 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["recheck_days"] = store.get_setting(story_id, "recheck_days")
story["last_full_recheck"] = store.get_setting(story_id, "last_full_recheck") story["last_full_recheck"] = store.get_setting(story_id, "last_full_recheck")
story["upload_held"] = bool(store.get_setting(story_id, "upload_held")) story["upload_held"] = bool(store.get_setting(story_id, "upload_held"))
story["needs_approval"] = _needs_approval(story_id) 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 { return {
"recent_chapters": _setting_recent_chapters(), "recent_chapters": _setting_recent_chapters(),
"uploads_paused": _uploads_paused(), **_upload_state(),
"ingest_available": config.INGEST_DIR is not None,
"link_books": _link_books(),
"library_matching_available": _can_match_books(),
"story": story, "story": story,
"chapters": store.list_chapters(story_id), "chapters": store.list_chapters(story_id),
"rules": store.rules(story_id),
"book": book, "book": book,
"library_available": library_available(),
"library_error": library_error, "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 queued = None
if story["metahtml"] and (story["calibre_id"] or story["ingested_at"]): 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() 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 # 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 ini = config.PERSONAL_INI
return { return {
"recent_chapters": _setting_recent_chapters(), "recent_chapters": _setting_recent_chapters(),
"link_books": _link_books(), **_upload_state(),
"linking_available": _linking_available(), "linking_available": _linking_available(),
"hold_new_links": bool(store.get_setting(None, "hold_new_links")), "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 "", "personal_ini": ini.read_text(encoding="utf-8") if ini.exists() else "",
**job_list_settings(), **job_list_settings(),
} }
@@ -433,7 +427,7 @@ def save_settings(
partial = config.PERSONAL_INI.with_name("personal.ini.part") partial = config.PERSONAL_INI.with_name("personal.ini.part")
partial.write_text(personal_ini, encoding="utf-8") partial.write_text(personal_ini, encoding="utf-8")
partial.replace(config.PERSONAL_INI) partial.replace(config.PERSONAL_INI)
if released and not _uploads_paused(): if released and _can_upload():
_release_held_uploads() _release_held_uploads()
return {**get_settings(), "warnings": warnings} 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"]: 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.") 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) 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 job_queue.add(Job(mode=DownloadMode.REBUILD, story_id=story_id, title=story["title"])).to_dict()
return None 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: 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.""" """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: if story["calibre_id"] is not None:
return {"id": story["calibre_id"], "uuid": story["calibre_uuid"], "title": story["title"]} return {"id": story["calibre_id"], "uuid": story["calibre_uuid"], "title": story["title"]}
return None 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. # with the next upload or an approval. "upload_held" marks a built book that did not go to CWA.
def _uploads_paused() -> bool: def _paused() -> bool:
return config.INGEST_DIR is None or bool(store.get_setting(None, "pause_uploads")) """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: 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: def _hold_reason(story: dict[str, Any], book: dict[str, Any] | None) -> str | None:
if _uploads_paused(): if config.INGEST_DIR is None:
if config.INGEST_DIR is None: return "Not sent. INGEST_DIR is not set."
return "Not sent. INGEST_DIR is not set." if _paused():
return "Not sent. Turn off pause to send." return "Not sent. Turn off pause to send."
if not _needs_approval(story["id"]): if not _needs_approval(story["id"]):
return None return None
+4 -4
View File
@@ -174,7 +174,7 @@ export function storyPage(main, { storyId }, config) {
}; };
const approveAsNew = async () => { 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 approve this story as a new book? Uploads remain paused. '
: 'Unlink the library book, and send this story to CWA as a new book? '; : 'Unlink the library book, and send this story to CWA as a new book? ';
const message = action + const message = action +
@@ -193,7 +193,7 @@ export function storyPage(main, { storyId }, config) {
const unlinked = !linking && story.calibre_id == null; const unlinked = !linking && story.calibre_id == null;
// A previously uploaded book needs a known target before approval. // A previously uploaded book needs a known target before approval.
const approval = linking && story.needs_approval && (story.calibre_id != null || !story.ingested_at); 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; if (!paused && !approval && !unlinked) return null;
let target = 'Approve this story as a new CWA book.'; let target = 'Approve this story as a new CWA book.';
if (book) { if (book) {
@@ -223,11 +223,11 @@ export function storyPage(main, { storyId }, config) {
h( h(
'div', 'div',
{ class: 'controls' }, { 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', class: 'primary',
}), }),
story.calibre_uuid && button( 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,
), ),
), ),
], ],
+3 -3
View File
@@ -368,7 +368,7 @@ def story_settings(story_id: int) -> Any:
data.get("auto_update"), data.get("auto_update"),
None if days is None else _int(days, "recheck_days"), 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/<int:story_id>/metadata") @app.put("/api/stories/<int:story_id>/metadata")
@@ -382,13 +382,13 @@ def edit_metadata(story_id: int) -> Any:
@app.post("/api/stories/<int:story_id>/link") @app.post("/api/stories/<int:story_id>/link")
def link(story_id: int) -> Any: def link(story_id: int) -> Any:
orchestrator.link_story(story_id, _json().get("uuid") or None) 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/<int:story_id>/approve") @app.post("/api/stories/<int:story_id>/approve")
def approve_upload(story_id: int) -> Any: def approve_upload(story_id: int) -> Any:
job = orchestrator.approve_upload(story_id, _json().get("uuid") or None) 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/<int:story_id>/chapters") @app.post("/api/stories/<int:story_id>/chapters")
+1 -1
View File
@@ -647,7 +647,7 @@ test('with CWA off, the pages show nothing about CWA', async () => {
assert.ok(section(main, 'Library link')); assert.ok(section(main, 'Library link'));
const off = { ...config, cwa_enabled: false, library_available: false }; const off = { ...config, cwa_enabled: false, library_available: false };
// Values that show a notice with CWA on. // 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; details.story.upload_held = true;
await mountStory(off); await mountStory(off);
assert.ok(section(main, 'Metadata')); assert.ok(section(main, 'Metadata'));
+9 -9
View File
@@ -15,7 +15,7 @@ const details = (story, extra = {}) => ({
calibre_id: null, calibre_uuid: null, ingested_at: null, upload_held: false, needs_approval: false, ...story }, calibre_id: null, calibre_uuid: null, ingested_at: null, upload_held: false, needs_approval: false, ...story },
chapters: [], chapters: [],
recent_chapters: 0, recent_chapters: 0,
uploads_paused: false, pause_uploads: false,
book: null, book: null,
...extra, ...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) => { test(`approval while paused saves the decision without promising an upload: ${uuid}`, async (t) => {
const data = details( const data = details(
{ calibre_id: uuid ? 7 : null, calibre_uuid: uuid, upload_held: true, needs_approval: true }, { 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); const { backend, notice } = await renderStory(t, data);
assert.ok(find(notice, 'button', 'Approve')); 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) => { test('approval as a new book while paused keeps the pause in the confirmation', async (t) => {
const data = details( const data = details(
{ calibre_id: 7, calibre_uuid: 'book-7', upload_held: true, needs_approval: true }, { 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 { backend, notice } = await renderStory(t, data);
const confirm = t.mock.method(window, 'confirm', () => true); 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) => { test('a paused upload points to the settings', async (t) => {
const { notice } = await renderStory( const { notice } = await renderStory(
t, 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.match(notice.textContent, /Uploads to CWA are paused/);
assert.equal(find(notice, 'a', 'Settings').attributes.href, '#/settings'); 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( const { notice } = await renderStory(t, details(
{ calibre_id: savedId, calibre_uuid: savedId ? 'book-7' : null, { calibre_id: savedId, calibre_uuid: savedId ? 'book-7' : null,
upload_held: true, needs_approval: needsApproval }, upload_held: true, needs_approval: needsApproval },
{ link_books: false, uploads_paused: paused }, { link_books: false, pause_uploads: paused },
)); ));
if (savedId !== null && !paused) { if (savedId !== null && !paused) {
assert.equal(notice, undefined); 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) => { test('missing INGEST_DIR points to the environment rather than the disabled pause switch', async (t) => {
const { notice } = await renderStory(t, details( 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(notice.textContent, 'Set INGEST_DIR and restart Bindery to enable uploads.');
assert.equal(nodes(notice).filter((el) => el.tagName === 'button').length, 0); 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) => { test('missing library sources leave uploads enabled with the duplicate warning', async (t) => {
const { notice } = await renderStory(t, details( const { notice } = await renderStory(t, details(
{ ingested_at: '2026-01-01', upload_held: true, needs_approval: true }, { 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.match(notice.textContent, /If CWA imports them, updates will create duplicate books/);
assert.equal(nodes(notice).filter((el) => el.tagName === 'button').length, 0); 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) => { test('a new book asks for approval when library linking is enabled', async (t) => {
const { notice } = await renderStory(t, details( const { notice } = await renderStory(t, details(
{ upload_held: true, needs_approval: true }, { 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.match(notice.textContent, /^Approve this story as a new CWA book\./);
assert.ok(find(notice, 'button', 'Approve and send')); 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) => { test('approval before a build still shows the pause and never offers to send', async (t) => {
const { notice } = await renderStory(t, details( 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.match(notice.textContent, /Uploads to CWA are paused\. Resume them in Settings\./);
assert.ok(find(notice, 'button', 'Approve')); assert.ok(find(notice, 'button', 'Approve'));
+12
View File
@@ -187,3 +187,15 @@ def test_chapter_requests_without_ids_name_what_to_select(client, story):
for body in ({}, {"chapter_ids": None}): for body in ({}, {"chapter_ids": None}):
response = client.post(f"/api/stories/{sid}/chapters/delete", json=body, headers=H) 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") 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()
+2 -3
View File
@@ -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 list(config.TMP_DIR.glob(f"story-{story_id}-*.epub"))
assert not list((tmp_path / "ingest").iterdir()) assert not list((tmp_path / "ingest").iterdir())
details = orchestrator.story_details(story_id) 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["story"]["upload_held"] is True
assert details["library_error"] is None 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"]["upload_held"] is False
assert details["story"]["needs_approval"] is False assert details["story"]["needs_approval"] is False
assert details["link_books"] is False assert details["link_books"] is False
assert details["library_matching_available"] is False
assert details["ingest_available"] is True assert details["ingest_available"] is True
monkeypatch.setattr(config, "INGEST_DIR", None) monkeypatch.setattr(config, "INGEST_DIR", None)
assert client.get(f"/api/stories/{story_id}").json["ingest_available"] is False assert client.get(f"/api/stories/{story_id}").json["ingest_available"] is False
monkeypatch.setattr(config, "CWA_URL", "https://cwa.invalid") 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]) @pytest.mark.parametrize("missing_ingest", [False, True])