From ca3d7301294baed4fa66e6617668ad0a28824681 Mon Sep 17 00:00:00 2001 From: Timothy Allen Date: Tue, 6 Oct 2026 16:59:15 +0200 Subject: [PATCH] Turn CWA off unless CWA_ENABLED is true CWA_ENABLED now defaults to false. If a CWA variable is set while CWA is off, the start log has a warning that names it. On the Settings page, the alert about INGEST_DIR moves to Book Downloads and shows with CWA on or off. The Uploads to CWA card moves below Book Downloads. --- bindery/auth.py | 2 +- bindery/config.py | 13 +++--- bindery/static/js/settings.js | 74 +++++++++++++++++------------------ bindery/static/js/story.js | 2 +- tests/conftest.py | 1 + tests/js/flows.test.mjs | 20 +++++++--- tests/js/upload-hold.test.mjs | 4 +- tests/test_config.py | 25 +++++++----- tests/test_optional_cwa.py | 20 +++++++--- 9 files changed, 93 insertions(+), 68 deletions(-) diff --git a/bindery/auth.py b/bindery/auth.py index 860a4b9..14b8483 100644 --- a/bindery/auth.py +++ b/bindery/auth.py @@ -123,7 +123,7 @@ def config_error() -> str | None: if not _valid_local_hash(config.LOCAL_PASSWORD_HASH): return "BINDERY_PASSWORD_HASH must be a Werkzeug scrypt or PBKDF2 hash of a nonempty password" if method == AUTH_CWA and not config.CWA_ENABLED: - return "AUTH_METHOD=cwa needs CWA, but CWA_ENABLED is false" + return "AUTH_METHOD=cwa needs CWA_ENABLED=true" if method == AUTH_CWA and not (config.CWA_URL or config.CWA_DB_PATH): return "AUTH_METHOD=cwa needs CWA_URL or CWA_DB_PATH" if method == AUTH_PROXY: diff --git a/bindery/config.py b/bindery/config.py index 3800571..ca1d860 100644 --- a/bindery/config.py +++ b/bindery/config.py @@ -74,9 +74,9 @@ CWA_PASSWORD = os.getenv("CWA_PASSWORD", "") # Independent, optional read-only databases. HTTP remains the fallback. CWA_DB_PATH = _optional_path("CWA_DB_PATH") # Calibre-Web app.db, for logins CALIBRE_DB_PATH = _optional_path("CALIBRE_DB_PATH") # calibre metadata.db, for book IDs -# false if the user has no CWA. Bindery then ignores the CWA settings above, and shows nothing about CWA. INGEST_DIR -# still gets each built book, as a plain epub. Only a clear "false" turns CWA off. -CWA_ENABLED = _bool("CWA_ENABLED", True) +# true if the user has CWA. Without it, Bindery ignores the CWA settings above, and shows nothing about CWA. INGEST_DIR +# still gets each built book, as a plain epub. Only a clear "true" turns CWA on. +CWA_ENABLED = _bool("CWA_ENABLED") CWA_SETTINGS = ("CWA_URL", "CWA_USERNAME", "CWA_PASSWORD", "CWA_DB_PATH", "CALIBRE_DB_PATH") IGNORED_SETTINGS = [] if CWA_ENABLED else [name for name in CWA_SETTINGS if os.getenv(name, "").strip()] if not CWA_ENABLED: @@ -141,7 +141,10 @@ def prepare() -> None: logger.info(f"Created the default FanFicFare configuration at {PERSONAL_INI}") if not CWA_ENABLED: if IGNORED_SETTINGS: - logger.info(f"CWA_ENABLED is false. Bindery ignores {', '.join(IGNORED_SETTINGS)}.") + logger.warning( + f"CWA_ENABLED is not true. Bindery ignores {', '.join(IGNORED_SETTINGS)}. To use CWA, set " + "CWA_ENABLED=true." + ) return if CWA_PASSWORD and not CWA_USERNAME: logger.warning("CWA_PASSWORD is set, but CWA_USERNAME is not. Bindery sends no login to CWA.") @@ -149,7 +152,7 @@ def prepare() -> None: for warning in missing: logger.warning(warning) if missing: - logger.warning("If you do not use CWA, set CWA_ENABLED=false. Bindery then shows no messages about CWA.") + logger.warning("If you do not use CWA, remove CWA_ENABLED. Bindery then shows no messages about CWA.") def _missing_cwa() -> list[str]: diff --git a/bindery/static/js/settings.js b/bindery/static/js/settings.js index 05cd2f1..9308e96 100644 --- a/bindery/static/js/settings.js +++ b/bindery/static/js/settings.js @@ -1,4 +1,4 @@ -// Settings: the recheck of recent chapters, the uploads to CWA, the names of downloaded books, the global rules, +// Settings: the recheck of recent chapters, the names of downloaded books, the uploads to CWA, the global rules, // and FanFicFare's personal.ini. import { api, button, checkbox, externalLink, fill, h, showError, toast } from './lib.js'; @@ -39,7 +39,7 @@ export function settingsPage(main, _params, config) { const linkBooks = checkbox(true, showLinkState); const pauseUploads = checkbox(false, () => {}); const ingestNote = h('p', { class: 'notice', id: 'ingest-note', hidden: true }, - 'INGEST_DIR is not set. Uploads stay paused until you set it and restart Bindery.'); + 'INGEST_DIR is not set. Bindery sends no book to the ingest folder until you set it and restart Bindery.'); const showUploadSettings = (settings) => { linkBooks.disabled = settings.linking_available === false; linkBooks.checked = !linkBooks.disabled && (settings.link_books ?? true); @@ -178,41 +178,6 @@ export function settingsPage(main, _params, config) { ), saveButton(() => ({ recent_chapters: recent.value === '' ? 0 : Number(recent.value) })), ), - config.cwa_enabled && card( - 'Uploads to CWA', - h('label', {}, linkBooks, ' Link books to CWA library'), - h( - 'p', - { class: 'hint' }, - 'Find library books through CWA_URL or CALIBRE_DB_PATH. When this is off, uploads use saved CWA book IDs ' + - 'or send a plain ePub to INGEST_DIR.', - ), - libraryNote, - linkNote, - h('label', {}, holdNewLinks, ' Hold new links for approval'), - h( - 'p', - { class: 'hint' }, - 'The first upload of a new story waits for your approval. So does the first upload after a change of the ' + - 'library link, also a link that you make. Check the linked book on the story page, and approve it there. ' + - 'Later updates of an approved book go to CWA as usual.', - ), - h('label', {}, pauseUploads, ' Pause all uploads'), - ingestNote, - h( - 'p', - { class: 'hint' }, - 'Bindery downloads chapters and builds books, but sends no book to CWA. ' + - 'When you turn the pause off, Bindery sends the books that wait.', - ), - saveButton( - () => ({ link_books: linkBooks.checked, hold_new_links: holdNewLinks.checked, pause_uploads: pauseUploads.checked }), - (settings) => { - showUploadSettings(settings); - toast('Saved'); - }, - ), - ), card( 'Book Downloads', h('label', {}, opdsEnabled, ' Enable OPDS'), @@ -331,6 +296,7 @@ export function settingsPage(main, _params, config) { '. The default gives "May, 2020". A / in a date becomes _, as in Calibre.', ), ), + ingestNote, saveButton( () => ({ opds_enabled: opdsEnabled.checked, @@ -346,6 +312,40 @@ export function settingsPage(main, _params, config) { }, ), ), + config.cwa_enabled && card( + 'Uploads to CWA', + h('label', {}, linkBooks, ' Link books to CWA library'), + h( + 'p', + { class: 'hint' }, + 'Find library books through CWA_URL or CALIBRE_DB_PATH. When this is off, uploads use saved CWA book IDs ' + + 'or send a plain ePub to INGEST_DIR.', + ), + libraryNote, + linkNote, + h('label', {}, holdNewLinks, ' Hold new links for approval'), + h( + 'p', + { class: 'hint' }, + 'The first upload of a new story waits for your approval. So does the first upload after a change of the ' + + 'library link, also a link that you make. Check the linked book on the story page, and approve it there. ' + + 'Later updates of an approved book go to CWA as usual.', + ), + h('label', {}, pauseUploads, ' Pause all uploads'), + h( + 'p', + { class: 'hint' }, + 'Bindery downloads chapters and builds books, but sends no book to CWA. ' + + 'When you turn the pause off, Bindery sends the books that wait.', + ), + saveButton( + () => ({ link_books: linkBooks.checked, hold_new_links: holdNewLinks.checked, pause_uploads: pauseUploads.checked }), + (settings) => { + showUploadSettings(settings); + toast('Saved'); + }, + ), + ), card( 'Jobs list', h( diff --git a/bindery/static/js/story.js b/bindery/static/js/story.js index 9c8ca00..8bb9c62 100644 --- a/bindery/static/js/story.js +++ b/bindery/static/js/story.js @@ -68,7 +68,7 @@ const table = (headings, rows, extra = '') => export function storyPage(main, { storyId }, config) { const id = Number(storyId); const fields = config.metadata_fields; - // With CWA_ENABLED=false, the page shows nothing about CWA. + // Without CWA_ENABLED=true, the page shows nothing about CWA. const cwa = config.cwa_enabled; const libraryStays = cwa ? ' The library book stays.' : ''; let details = null; diff --git a/tests/conftest.py b/tests/conftest.py index 80d60df..0e1a163 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -10,6 +10,7 @@ for name, sub in [("CONFIG_DIR", "config"), ("INGEST_DIR", "ingest"), ("TMP_DIR" (_tmp / "ingest").mkdir() # Bindery does not make the ingest folder os.environ["BINDERY_START_THREADS"] = "false" os.environ["UPDATE_INTERVAL_HOURS"] = "0" +os.environ["CWA_ENABLED"] = "true" # most tests are about a Bindery with CWA for name in ( "CWA_URL", "CWA_USERNAME", diff --git a/tests/js/flows.test.mjs b/tests/js/flows.test.mjs index 3fb630a..622b8f2 100644 --- a/tests/js/flows.test.mjs +++ b/tests/js/flows.test.mjs @@ -115,8 +115,8 @@ test('each group of settings is an open card, and the download help starts close const cards = main.children.filter((el) => el.tagName !== 'h1'); assert.deepEqual(cards.map((el) => [el.tagName, el.className, el.open, el.children[0].textContent]), [ ['details', 'card', true, 'Recheck recent chapters'], - ['details', 'card', true, 'Uploads to CWA'], ['details', 'card', true, 'Book Downloads'], + ['details', 'card', true, 'Uploads to CWA'], ['details', 'card', true, 'Jobs list'], ['details', 'card', true, 'Global replacement rules'], ['details', 'card', true, 'FanFicFare settings (personal.ini)'], @@ -789,9 +789,17 @@ test('with CWA off, the pages show nothing about CWA', async () => { assert.ok(section(main, 'Add stories')); assert.equal(section(main, 'Import from the library'), undefined); assert.doesNotMatch(main.textContent, /CWA|INGEST_DIR|library/i); - settingsPage(main, {}, off); - await settle(); - assert.ok(section(main, 'Jobs list')); - assert.equal(section(main, 'Uploads to CWA'), undefined); - assert.doesNotMatch(main.textContent, /CWA|INGEST_DIR|library/i); + for (const ingest of [true, false]) { + api.on('GET', 'settings', { recent_chapters: 3, personal_ini: '[defaults]', ingest_available: ingest }); + settingsPage(main, {}, off); + await settle(); + assert.ok(section(main, 'Jobs list')); + assert.equal(section(main, 'Uploads to CWA'), undefined); + assert.doesNotMatch(main.textContent, /CWA|library/i); + // The alert about INGEST_DIR is under Book Downloads, also with CWA off. + const note = nodes(section(main, 'Book Downloads')).find((el) => el.attributes.id === 'ingest-note'); + assert.equal(note.hidden, ingest); + assert.equal(note.className, 'notice'); + assert.match(note.textContent, /INGEST_DIR is not set\. Bindery sends no book to the ingest folder/); + } }); diff --git a/tests/js/upload-hold.test.mjs b/tests/js/upload-hold.test.mjs index 0cec324..defe75d 100644 --- a/tests/js/upload-hold.test.mjs +++ b/tests/js/upload-hold.test.mjs @@ -54,7 +54,7 @@ test('the upload switches load and save on the settings page', async (t) => { assert.equal(warning.hidden, true); assert.equal(link.attributes['aria-describedby'], ''); assert.match(warning.textContent, /Updates without a saved CWA book ID create duplicate books if CWA imports them/); - assert.equal(nodes(card).find((el) => el.attributes.id === 'ingest-note').hidden, true); + assert.equal(nodes(section(main, 'Book Downloads')).find((el) => el.attributes.id === 'ingest-note').hidden, true); assert.equal(pause.disabled, false); assert.equal(hold.disabled, false); assert.equal(link.disabled, false); @@ -129,7 +129,7 @@ for (const linkBooks of [true, false]) { assert.equal(nodes(card).find((el) => el.attributes.id === 'link-note').hidden, linkBooks); assert.equal(pause.checked, true); assert.equal(pause.disabled, true); - const note = nodes(card).find((el) => el.attributes.id === 'ingest-note'); + const note = nodes(section(main, 'Book Downloads')).find((el) => el.attributes.id === 'ingest-note'); assert.equal(note.hidden, false); assert.equal(note.className, 'notice'); assert.equal(pause.attributes['aria-describedby'], note.attributes.id); diff --git a/tests/test_config.py b/tests/test_config.py index d4f7b9f..408f5d0 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -143,7 +143,7 @@ def test_defaults(monkeypatch, reload_config): ] assert config.ALLOWED_HOSTS == [] assert config.ADMIN_USERS == [] - assert config.CWA_ENABLED is True + assert config.CWA_ENABLED is False assert config.IGNORED_SETTINGS == [] assert config.SESSION_COOKIE_SECURE is False assert config.TRUST_PROXY is False @@ -161,9 +161,12 @@ def test_empty_ingest_directory_disables_uploads(monkeypatch, reload_config, val assert config.INGEST_DIR is None -@pytest.mark.parametrize("value", ["false", " No ", "0", "off"]) +@pytest.mark.parametrize("value", [None, "", "false", " No ", "0", "maybe"]) def test_cwa_off_ignores_the_cwa_settings_but_not_the_ingest_folder(monkeypatch, reload_config, value): - monkeypatch.setenv("CWA_ENABLED", value) + if value is None: + monkeypatch.delenv("CWA_ENABLED") + else: + monkeypatch.setenv("CWA_ENABLED", value) monkeypatch.setenv("CWA_URL", "https://cwa.invalid") monkeypatch.setenv("CWA_USERNAME", "bindery") monkeypatch.setenv("CWA_PASSWORD", "secret") @@ -178,8 +181,8 @@ def test_cwa_off_ignores_the_cwa_settings_but_not_the_ingest_folder(monkeypatch, assert str(config.INGEST_DIR) == "/books" -@pytest.mark.parametrize("value", ["", "true", "maybe"]) -def test_only_a_clear_false_turns_cwa_off(monkeypatch, reload_config, value): +@pytest.mark.parametrize("value", ["true", " Yes ", "1", "on"]) +def test_only_a_clear_true_turns_cwa_on(monkeypatch, reload_config, value): monkeypatch.setenv("CWA_ENABLED", value) monkeypatch.setenv("CWA_URL", "https://cwa.invalid") reload_config() @@ -197,17 +200,19 @@ def test_prepare_with_cwa_off_logs_nothing_about_missing_cwa(folders, monkeypatc with caplog.at_level("INFO"): config.prepare() lines = [r.getMessage() for r in caplog.records] - expected = ["CWA_ENABLED is false. Bindery ignores CWA_URL, CALIBRE_DB_PATH."] if ignored else [] + ignores = "CWA_ENABLED is not true. Bindery ignores CWA_URL, CALIBRE_DB_PATH. To use CWA, set CWA_ENABLED=true." + expected = [ignores] if ignored else [] + assert [r.levelname for r in caplog.records if "CWA" in r.getMessage()] == ["WARNING"] * len(expected) assert [line for line in lines if "CWA" in line or "INGEST" in line] == expected @pytest.mark.parametrize( ("ingest", "url", "database", "expected"), [ - (False, "", None, ["INGEST_DIR is not set", "cannot find the library book of a story.", "CWA_ENABLED=false"]), - (True, "", None, ["so each update adds a new book to CWA.", "CWA_ENABLED=false"]), - (True, "", Path("/library/metadata.db"), ["CWA_URL is not set. Bindery cannot search", "CWA_ENABLED=false"]), - (False, "https://cwa.invalid", None, ["INGEST_DIR is not set", "CWA_ENABLED=false"]), + (False, "", None, ["INGEST_DIR is not set", "cannot find the library book of a story.", "remove CWA_ENABLED"]), + (True, "", None, ["so each update adds a new book to CWA.", "remove CWA_ENABLED"]), + (True, "", Path("/library/metadata.db"), ["CWA_URL is not set. Bindery cannot search", "remove CWA_ENABLED"]), + (False, "https://cwa.invalid", None, ["INGEST_DIR is not set", "remove CWA_ENABLED"]), (True, "https://cwa.invalid", None, []), ], ) diff --git a/tests/test_optional_cwa.py b/tests/test_optional_cwa.py index f5cfffe..ef0a3a3 100644 --- a/tests/test_optional_cwa.py +++ b/tests/test_optional_cwa.py @@ -40,7 +40,7 @@ def library(isolated, monkeypatch): monkeypatch.setattr(orchestrator, "_find_book", Mock(return_value=None)) -@pytest.mark.parametrize("enabled", [False, True]) +@pytest.mark.parametrize("enabled", [None, False, True]) def test_app_starts_without_any_cwa_configuration(tmp_path, enabled): env = { **os.environ, @@ -51,7 +51,14 @@ def test_app_starts_without_any_cwa_configuration(tmp_path, enabled): "URL_BASE": "", "CWA_ENABLED": str(enabled), } - for name in ("INGEST_DIR", "CWA_URL", "CWA_DB_PATH", "CALIBRE_DB_PATH"): + # CWA is off unless CWA_ENABLED turns it on. + for name in ( + "INGEST_DIR", + "CWA_URL", + "CWA_DB_PATH", + "CALIBRE_DB_PATH", + *(["CWA_ENABLED"] if enabled is None else []), + ): env.pop(name, None) result = subprocess.run( [ @@ -60,6 +67,7 @@ def test_app_starts_without_any_cwa_configuration(tmp_path, enabled): "from bindery.web import app; " "client = app.test_client(); " "assert client.get('/api/health').status_code == 200; " + f"assert client.get('/api/config').json['cwa_enabled'] is {bool(enabled)}; " "settings = client.get('/api/settings').json; " "assert settings['pause_uploads'] is True; " "assert settings['ingest_available'] is False; " @@ -73,9 +81,9 @@ def test_app_starts_without_any_cwa_configuration(tmp_path, enabled): ) assert result.returncode == 0, result.stderr assert "ERROR" not in result.stderr - # Bindery names what is missing, until the user says that there is no CWA. - for text in ("WARNING", "INGEST_DIR is not set", "CWA_URL and CALIBRE_DB_PATH are not set", "CWA_ENABLED=false"): - assert (text in result.stderr) is enabled + # Bindery names what is missing only for a user who turned CWA on. + for text in ("WARNING", "INGEST_DIR is not set", "CWA_URL and CALIBRE_DB_PATH are not set", "remove CWA_ENABLED"): + assert (text in result.stderr) is bool(enabled) if not enabled: assert "CWA" not in result.stderr and "INGEST" not in result.stderr @@ -134,7 +142,7 @@ def test_cwa_off_shows_in_the_config_and_refuses_cwa_logins(client, monkeypatch, monkeypatch.setattr(config, "CWA_DB_PATH", "/cwa/app.db") response = client.get("/api/stories") assert response.status_code == 503 - assert response.json["error"].startswith("AUTH_METHOD=cwa needs CWA, but CWA_ENABLED is false.") + assert response.json["error"].startswith("AUTH_METHOD=cwa needs CWA_ENABLED=true.") def test_without_cwa_the_download_template_uses_the_story_id(client, cwa_off):