mirror of
https://github.com/Second-Hand-Friends/kleinanzeigen-bot.git
synced 2026-03-12 10:31:50 +01:00
fix: reject invalid --ads selector values instead of silent fallback (#834)
## Summary - When an invalid `--ads` value is explicitly provided (e.g. `--ads=my-directory-name`), the bot now exits with code 2 and a clear error message listing valid options, instead of silently falling back to the command default - Fixes the numeric ID regex from unanchored `\d+[,\d+]*` (which could match partial strings like `abc123`) to anchored `^\d+(,\d+)*$` - Adds `_is_valid_ads_selector()` helper to deduplicate validation logic across publish/update/download/extend commands ## Motivation When calling `kleinanzeigen-bot publish --ads=led-grow-light-set` (passing a directory name instead of a numeric ad ID), the bot silently fell back to `--ads=due` and republished all due ads — causing unintended republication of multiple ads and loss of conversations on those ads. The silent fallback with only a WARNING log message makes it too easy to accidentally trigger unwanted operations. An explicit error with exit code 2 (consistent with other argument validation like `--workspace-mode`) is the expected behavior for invalid arguments. ## Changes | File | Change | |------|--------| | `src/kleinanzeigen_bot/__init__.py` | Added `_ads_selector_explicit` flag (set when `--ads` or `--force` is used), `_is_valid_ads_selector()` helper method, and updated all 4 command blocks (publish/update/download/extend) to error on explicitly invalid selectors | | `resources/translations.de.yaml` | Replaced 3 old fallback messages with 4 new error messages | | `tests/unit/test_init.py` | Updated 2 existing tests to expect `SystemExit(2)` instead of silent fallback, added 2 new tests for update/extend invalid selectors | ## Test plan - [x] All 754 unit tests pass (`pdm run utest`) - [x] Lint clean (`pdm run lint`) - [x] Translation completeness verified (`test_all_log_messages_have_translations`, `test_no_obsolete_translations`) - [x] `--ads=invalid` on publish/update/download/extend all exit with code 2 - [x] Default behavior (no `--ads` flag) unchanged for all commands - [x] Valid selectors (`--ads=all`, `--ads=due`, `--ads=12345,67890`, `--ads=changed,due`) still work <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Stricter validation of ad selectors; invalid selectors now terminate with exit code 2 and preserve safe defaults when no selector is provided. * **New Features** * Support for comma-separated numeric ID lists as a valid selector format. * **Tests** * Unit tests updated to assert non-zero exit on invalid selectors and verify default-fallback behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Liermann Torsten - Hays <liermann.hays@partner.akdb.de> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1152,6 +1152,20 @@ class TestKleinanzeigenBotAdOperations:
|
||||
await test_bot.run(["script.py", "download"])
|
||||
assert test_bot.ads_selector == "new"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_update_default_selector(self, test_bot:KleinanzeigenBot, mock_config_setup:None) -> None: # pylint: disable=unused-argument
|
||||
"""Test running update command with default selector falls back to changed."""
|
||||
with patch.object(test_bot, "load_ads", return_value = []):
|
||||
await test_bot.run(["script.py", "update"])
|
||||
assert test_bot.ads_selector == "changed"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_extend_default_selector(self, test_bot:KleinanzeigenBot, mock_config_setup:None) -> None: # pylint: disable=unused-argument
|
||||
"""Test running extend command with default selector falls back to all."""
|
||||
with patch.object(test_bot, "load_ads", return_value = []):
|
||||
await test_bot.run(["script.py", "extend"])
|
||||
assert test_bot.ads_selector == "all"
|
||||
|
||||
def test_load_ads_no_files(self, test_bot:KleinanzeigenBot) -> None:
|
||||
"""Test loading ads with no files."""
|
||||
test_bot.config.ad_files = ["nonexistent/*.yaml"]
|
||||
@@ -1172,17 +1186,31 @@ class TestKleinanzeigenBotAdManagement:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_publish_invalid_selector(self, test_bot:KleinanzeigenBot, mock_config_setup:None) -> None: # pylint: disable=unused-argument
|
||||
"""Test running publish with invalid selector."""
|
||||
with patch.object(test_bot, "load_ads", return_value = []):
|
||||
"""Test running publish with invalid selector exits with error."""
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
await test_bot.run(["script.py", "publish", "--ads=invalid"])
|
||||
assert test_bot.ads_selector == "due"
|
||||
assert exc_info.value.code == 2
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_download_invalid_selector(self, test_bot:KleinanzeigenBot, mock_config_setup:None) -> None: # pylint: disable=unused-argument
|
||||
"""Test running download with invalid selector."""
|
||||
with patch.object(test_bot, "download_ads", new_callable = AsyncMock):
|
||||
"""Test running download with invalid selector exits with error."""
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
await test_bot.run(["script.py", "download", "--ads=invalid"])
|
||||
assert test_bot.ads_selector == "new"
|
||||
assert exc_info.value.code == 2
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_update_invalid_selector(self, test_bot:KleinanzeigenBot, mock_config_setup:None) -> None: # pylint: disable=unused-argument
|
||||
"""Test running update with invalid selector exits with error."""
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
await test_bot.run(["script.py", "update", "--ads=invalid"])
|
||||
assert exc_info.value.code == 2
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_extend_invalid_selector(self, test_bot:KleinanzeigenBot, mock_config_setup:None) -> None: # pylint: disable=unused-argument
|
||||
"""Test running extend with invalid selector exits with error."""
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
await test_bot.run(["script.py", "extend", "--ads=invalid"])
|
||||
assert exc_info.value.code == 2
|
||||
|
||||
|
||||
class TestKleinanzeigenBotAdConfiguration:
|
||||
|
||||
Reference in New Issue
Block a user