fix: harden pickup shipping probe (#1054)

## ℹ️ Description
Fixes issue #1003 by distinguishing genuine pickup-only categories from
partially rendered shipping sections.

- Link to the related issue(s): Fix #1003
- Describe the motivation and context for this change.
  - Avoid silently treating a partially loaded page as PICKUP-only.

## 📋 Changes Summary
- Probe `#ad-shipping-enabled-no` first.
- If missing, probe `#ad-shipping-enabled` and only short-circuit when
both are absent.
- Raise a diagnostic `TimeoutError` when the fieldset exists but the
pickup radio is missing.
- Update unit tests and the German translation string.

### ⚙️ Type of Change
- [x] 🐞 Bug fix (non-breaking change which fixes an issue)
- [ ]  New feature (adds new functionality without breaking existing
usage)
- [ ] 💥 Breaking change (changes that might break existing user setups,
scripts, or configurations)

##  Checklist
- [x] I have reviewed my changes to ensure they meet the project's
standards.
- [x] I have tested my changes and ensured that all tests pass (`pdm run
test`).
- [x] I have formatted the code (`pdm run format`).
- [ ] I have verified that linting passes (`pdm run lint`).
- [x] I have updated documentation where necessary.

Note: `pdm run lint` currently reports existing `PLW0717` violations in
this checkout, so CI lint is expected to fail until that separate
cleanup is done.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved error handling for PICKUP shipping selection. The bot now
clearly distinguishes between page loading issues and categories without
shipping options, providing better error messages when the page may not
be fully loaded.

<!-- review_stack_entry_start -->

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/Second-Hand-Friends/kleinanzeigen-bot/pull/1054?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

<!-- review_stack_entry_end -->

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Jens
2026-05-24 14:47:33 +02:00
committed by GitHub
parent d9a4d22f2d
commit 0ea0fd5f0c
3 changed files with 43 additions and 11 deletions

View File

@@ -3404,13 +3404,16 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
async def __set_shipping(self, ad_cfg:Ad, mode:AdUpdateStrategy = AdUpdateStrategy.REPLACE) -> None:
short_timeout = self._timeout("quick_dom")
if ad_cfg.shipping_type == "PICKUP":
# Some categories (notably books 76/77 and comics 76/77/15156) render the
# "Eigenschaften und Versand" fieldset without a shipping enable/disable toggle —
# those ads are PICKUP-only by site convention, so the absence of the radio is
# the expected state and matches the configured shipping_type.
pickup_radio = await self.web_probe(By.ID, "ad-shipping-enabled-no", timeout = short_timeout)
if pickup_radio is None:
LOG.debug("PICKUP: no shipping toggle for this category; treating as already PICKUP.")
shipping_fieldset = await self.web_probe(By.ID, "ad-shipping-enabled", timeout = short_timeout)
if shipping_fieldset is not None:
raise TimeoutError(
_("Shipping fieldset is rendered, but the pickup radio is missing; page may not be fully loaded.")
)
# Some categories (notably books 76/77 and comics 76/77/15156) render no
# shipping fieldset at all — those ads are PICKUP-only by site convention.
LOG.debug("PICKUP: no shipping fieldset for this category; treating as already PICKUP.")
return
try:
if not await self.web_check(By.ID, "ad-shipping-enabled-no", Is.SELECTED, timeout = short_timeout):

View File

@@ -337,6 +337,7 @@ kleinanzeigen_bot/__init__.py:
__set_shipping:
"Failed to set shipping attribute for type '%s'!": "Fehler beim setzen des Versandattributs für den Typ '%s'!"
"Shipping fieldset is rendered, but the pickup radio is missing; page may not be fully loaded.": "Das Versand-Fieldset wird gerendert, aber die Abholungs-Option fehlt; die Seite wurde möglicherweise nicht vollständig geladen."
? "Shipping options dialog entry not found. Legacy '.versand_s' select UI is no longer supported and requires dedicated rebuild."
: "Einstieg in den Versandoptionen-Dialog nicht gefunden. Die alte '.versand_s'-Select-UI wird nicht mehr unterstützt und muss gezielt neu umgesetzt werden."
"Unable to open shipping options dialog!": "Versandoptionen-Dialog konnte nicht geöffnet werden!"

View File

@@ -4234,12 +4234,14 @@ class TestShippingDialogFlow:
ad_cfg = Ad.model_validate(base_ad_config | {"shipping_type": "PICKUP"})
with (
patch.object(test_bot, "web_probe", new_callable = AsyncMock, return_value = MagicMock()),
patch.object(test_bot, "web_probe", new_callable = AsyncMock, return_value = MagicMock()) as mock_probe,
patch.object(test_bot, "web_check", new_callable = AsyncMock, return_value = selected),
patch.object(test_bot, "web_click", new_callable = AsyncMock) as mock_click,
):
await getattr(test_bot, "_KleinanzeigenBot__set_shipping")(ad_cfg)
mock_probe.assert_awaited_once()
assert mock_probe.call_args.args[:2] == (By.ID, "ad-shipping-enabled-no")
if selected:
mock_click.assert_not_awaited()
else:
@@ -4264,20 +4266,46 @@ class TestShippingDialogFlow:
test_bot:KleinanzeigenBot,
base_ad_config:dict[str, Any],
) -> None:
"""Categories without a shipping toggle (e.g. books 76/77, comics 76/77/15156) are
PICKUP-only by site convention — the absence of ``ad-shipping-enabled-no`` should
"""Categories without a shipping fieldset (e.g. books 76/77, comics 76/77/15156)
are PICKUP-only by site convention — the absence of both shipping selectors should
short-circuit without calling ``web_check``/``web_click``."""
ad_cfg = Ad.model_validate(base_ad_config | {"shipping_type": "PICKUP"})
with (
patch.object(test_bot, "web_probe", new_callable = AsyncMock, return_value = None) as mock_probe,
patch.object(test_bot, "web_probe", new_callable = AsyncMock, side_effect = [None, None]),
patch.object(test_bot, "web_check", new_callable = AsyncMock) as mock_check,
patch.object(test_bot, "web_click", new_callable = AsyncMock) as mock_click,
):
await getattr(test_bot, "_KleinanzeigenBot__set_shipping")(ad_cfg)
mock_probe.assert_awaited_once()
assert mock_probe.call_args.args[:2] == (By.ID, "ad-shipping-enabled-no")
mock_check.assert_not_awaited()
mock_click.assert_not_awaited()
@pytest.mark.asyncio
async def test_pickup_shipping_raises_when_fieldset_rendered_but_pickup_radio_missing(
self,
test_bot:KleinanzeigenBot,
base_ad_config:dict[str, Any],
) -> None:
"""A rendered shipping fieldset without the pickup radio should be treated as an error."""
ad_cfg = Ad.model_validate(base_ad_config | {"shipping_type": "PICKUP"})
with (
patch.object(test_bot, "web_probe", new_callable = AsyncMock, side_effect = [None, MagicMock()]) as mock_probe,
patch.object(test_bot, "web_check", new_callable = AsyncMock) as mock_check,
patch.object(test_bot, "web_click", new_callable = AsyncMock) as mock_click,
pytest.raises(
TimeoutError,
match = "Shipping fieldset is rendered, but the pickup radio is missing; page may not be fully loaded.",
),
):
await getattr(test_bot, "_KleinanzeigenBot__set_shipping")(ad_cfg)
assert mock_probe.await_count == 2
assert [call.args[:2] for call in mock_probe.await_args_list] == [
(By.ID, "ad-shipping-enabled-no"),
(By.ID, "ad-shipping-enabled"),
]
mock_check.assert_not_awaited()
mock_click.assert_not_awaited()