mirror of
https://github.com/Second-Hand-Friends/kleinanzeigen-bot.git
synced 2026-07-09 20:51:05 +02:00
refactor: extract publishing form helpers (#1129)
## ℹ️ Description - Extract browser-independent publishing form helpers from the monolith into `ad_form_helpers.py`. - Prepares later publishing form extraction without changing browser behavior. ## 📋 Changes Summary - Move the Versand combobox selector constant to `ad_form_helpers.py`. - Move location matching logic to `ad_form_helpers.py`. - Update production call sites and helper ownership tests. - Keep form sections and browser-dependent logic in `__init__.py`. ### ⚙️ Type of Change - [ ] 🐞 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`). - [x] I have verified that linting passes (`pdm run lint`). - [x] I have updated documentation where necessary. By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Improved internal code organization by consolidating shared helpers for form-related utilities and location matching logic. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -61,12 +61,6 @@ _LOGGED_OUT_CTA_SELECTORS:Final[list[tuple["By", str]]] = [
|
||||
(By.CSS_SELECTOR, 'a[href*="einloggen"]'),
|
||||
(By.CSS_SELECTOR, 'a[href*="/m-einloggen"]'),
|
||||
]
|
||||
_VERSAND_COMBOBOX_SELECTOR:Final[str] = (
|
||||
'button[role="combobox"][id="versand"], '
|
||||
'button[role="combobox"][id$=".versand"], '
|
||||
'button[role="combobox"][aria-labelledby$="versand-selected-option"]'
|
||||
)
|
||||
|
||||
colorama.just_fix_windows_console()
|
||||
|
||||
|
||||
@@ -1032,7 +1026,7 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
|
||||
try:
|
||||
shipping_btn = await self.web_find(
|
||||
By.CSS_SELECTOR,
|
||||
_VERSAND_COMBOBOX_SELECTOR,
|
||||
_ad_form_helpers.VERSAND_COMBOBOX_SELECTOR,
|
||||
timeout = self.timeout("quick_dom"),
|
||||
)
|
||||
btn_id = cast(str, shipping_btn.attrs.get("id"))
|
||||
@@ -1194,28 +1188,6 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
|
||||
ad_cfg.title, ad_id, exc_info = True,
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def _location_matches_target(target:str, candidate:str | None) -> bool:
|
||||
if not candidate:
|
||||
return False
|
||||
|
||||
normalized_target = " ".join(target.split()).casefold()
|
||||
normalized_candidate = " ".join(candidate.split()).casefold()
|
||||
if not normalized_target or not normalized_candidate:
|
||||
return False
|
||||
|
||||
if normalized_target == normalized_candidate:
|
||||
return True
|
||||
|
||||
if " - " in normalized_target:
|
||||
return False
|
||||
|
||||
if normalized_candidate.startswith(f"{normalized_target} - "):
|
||||
return True
|
||||
|
||||
candidate_city = normalized_candidate.rsplit(" - ", maxsplit = 1)[-1]
|
||||
return normalized_target == candidate_city
|
||||
|
||||
async def _city_option_text(self, option:Element) -> str:
|
||||
text = str(getattr(option, "text", "") or "").strip()
|
||||
if text:
|
||||
@@ -1324,7 +1296,7 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
|
||||
|
||||
async def _selection_converged() -> bool:
|
||||
selected_city = await self._read_city_selection_text()
|
||||
return self._location_matches_target(target, selected_city)
|
||||
return _ad_form_helpers.location_matches_target(target, selected_city)
|
||||
|
||||
try:
|
||||
await self.web_await(_selection_converged, timeout = city_flow_timeout)
|
||||
@@ -1337,7 +1309,7 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
|
||||
return
|
||||
|
||||
selected_city = await self._read_city_selection_text()
|
||||
if self._location_matches_target(target, selected_city):
|
||||
if _ad_form_helpers.location_matches_target(target, selected_city):
|
||||
return
|
||||
|
||||
city_timeout = self.timeout("default")
|
||||
@@ -1910,7 +1882,7 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
|
||||
# PostListingForm Astro island. In that UI the placeholder ("Bitte wählen")
|
||||
# must be replaced directly with either "Versand möglich" (value "ja") or
|
||||
# "Nur Abholung" (value "nein").
|
||||
shipping_combobox = await self.web_probe(By.CSS_SELECTOR, _VERSAND_COMBOBOX_SELECTOR, timeout = short_timeout)
|
||||
shipping_combobox = await self.web_probe(By.CSS_SELECTOR, _ad_form_helpers.VERSAND_COMBOBOX_SELECTOR, timeout = short_timeout)
|
||||
if shipping_combobox is not None:
|
||||
try:
|
||||
btn_id = cast(str, shipping_combobox.attrs.get("id"))
|
||||
|
||||
@@ -13,15 +13,23 @@ from kleinanzeigen_bot.model.ad_model import validate_condition_api_mapping
|
||||
__all__ = [
|
||||
"CONDITION_GERMAN_TO_API",
|
||||
"SPECIAL_ATTRIBUTE_TOKEN_RE",
|
||||
"VERSAND_COMBOBOX_SELECTOR",
|
||||
"WANTED_SHIPPING_LABELS",
|
||||
"get_marker_value",
|
||||
"get_marker_value_from_attrs",
|
||||
"location_matches_target",
|
||||
"normalize_condition",
|
||||
"xpath_literal",
|
||||
]
|
||||
|
||||
SPECIAL_ATTRIBUTE_TOKEN_RE:Final[re.Pattern[str]] = re.compile(r"^[A-Za-z0-9_]+$")
|
||||
|
||||
VERSAND_COMBOBOX_SELECTOR:Final[str] = (
|
||||
'button[role="combobox"][id="versand"], '
|
||||
'button[role="combobox"][id$=".versand"], '
|
||||
'button[role="combobox"][aria-labelledby$="versand-selected-option"]'
|
||||
)
|
||||
|
||||
WANTED_SHIPPING_LABELS:Final[dict[str, str]] = {
|
||||
"SHIPPING": "Versand möglich",
|
||||
"PICKUP": "Nur Abholung",
|
||||
@@ -53,6 +61,41 @@ def normalize_condition(condition_value:str) -> tuple[str, str | None]:
|
||||
return condition_value, None
|
||||
|
||||
|
||||
def location_matches_target(target:str, candidate:str | None) -> bool:
|
||||
"""Check if a city candidate matches the target location.
|
||||
|
||||
Returns ``True`` if the candidate (as displayed in a city combobox) matches
|
||||
the given target location. Handles zip-code prefixes (``"10115 - Berlin"``),
|
||||
whitespace normalization, and case folding.
|
||||
|
||||
Args:
|
||||
target: The expected location string (e.g. ``"Berlin"`` or ``"10115 - Berlin"``).
|
||||
candidate: The candidate string from a city combobox option, or ``None``.
|
||||
|
||||
Returns:
|
||||
``True`` if the candidate matches the target.
|
||||
"""
|
||||
if not candidate:
|
||||
return False
|
||||
|
||||
normalized_target = " ".join(target.split()).casefold()
|
||||
normalized_candidate = " ".join(candidate.split()).casefold()
|
||||
if not normalized_target or not normalized_candidate:
|
||||
return False
|
||||
|
||||
if normalized_target == normalized_candidate:
|
||||
return True
|
||||
|
||||
if " - " in normalized_target:
|
||||
return False
|
||||
|
||||
if normalized_candidate.startswith(f"{normalized_target} - "):
|
||||
return True
|
||||
|
||||
candidate_city = normalized_candidate.rsplit(" - ", maxsplit = 1)[-1]
|
||||
return normalized_target == candidate_city
|
||||
|
||||
|
||||
def xpath_literal(value:str) -> str:
|
||||
"""Return an XPath-safe string literal for *value*.
|
||||
|
||||
|
||||
@@ -13,6 +13,7 @@ from kleinanzeigen_bot.ad_form_helpers import (
|
||||
WANTED_SHIPPING_LABELS,
|
||||
get_marker_value,
|
||||
get_marker_value_from_attrs,
|
||||
location_matches_target,
|
||||
normalize_condition,
|
||||
xpath_literal,
|
||||
)
|
||||
@@ -140,6 +141,28 @@ def test_wanted_shipping_labels_exact_dict() -> None:
|
||||
}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# location_matches_target
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("target", "candidate", "expected"),
|
||||
[
|
||||
("10115 - Metroville", "10115 - Metroville", True),
|
||||
("10115 - Metroville", "12623 - Metroville", False),
|
||||
("Metroville", "12623 - Metroville", True),
|
||||
("Berlin", "Berlin - Mitte", True),
|
||||
("Metroville", None, False),
|
||||
("Berlin", "Hamburg", False),
|
||||
("Berlin", "berlin", True),
|
||||
("Berlin", " Berlin ", True),
|
||||
],
|
||||
)
|
||||
def test_location_matches_target(target:str, candidate:str | None, expected:bool) -> None:
|
||||
assert location_matches_target(target, candidate) is expected
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# CONDITION_GERMAN_TO_API
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -12,7 +12,6 @@ import pytest
|
||||
from nodriver.core.connection import ProtocolException
|
||||
|
||||
from kleinanzeigen_bot import (
|
||||
_VERSAND_COMBOBOX_SELECTOR, # noqa: PLC2701 - keep tests aligned with production selector
|
||||
LOG,
|
||||
SUBMISSION_MAX_RETRIES,
|
||||
KleinanzeigenBot,
|
||||
@@ -21,6 +20,7 @@ from kleinanzeigen_bot import (
|
||||
runtime_config,
|
||||
)
|
||||
from kleinanzeigen_bot._version import __version__
|
||||
from kleinanzeigen_bot.ad_form_helpers import VERSAND_COMBOBOX_SELECTOR
|
||||
from kleinanzeigen_bot.model.ad_model import Ad, AdUpdateStrategy
|
||||
from kleinanzeigen_bot.model.config_model import (
|
||||
AutoPriceReductionConfig,
|
||||
@@ -1630,23 +1630,6 @@ class TestDisplayCounterProgression:
|
||||
|
||||
|
||||
class TestKleinanzeigenBotContactLocationHardening:
|
||||
@pytest.mark.parametrize(
|
||||
("target", "candidate", "expected"),
|
||||
[
|
||||
("10115 - Metroville", "10115 - Metroville", True),
|
||||
("10115 - Metroville", "12623 - Metroville", False),
|
||||
("Metroville", "12623 - Metroville", True),
|
||||
("Berlin", "Berlin - Mitte", True),
|
||||
("Metroville", None, False),
|
||||
("Berlin", "Hamburg", False),
|
||||
("Berlin", "berlin", True),
|
||||
("Berlin", " Berlin ", True),
|
||||
],
|
||||
)
|
||||
def test_location_matches_target(self, test_bot:KleinanzeigenBot, target:str, candidate:str | None, expected:bool) -> None:
|
||||
matcher = getattr(test_bot, "_location_matches_target")
|
||||
assert matcher(target, candidate) is expected
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_city_selection_text_prefers_live_input_value(self, test_bot:KleinanzeigenBot) -> None:
|
||||
city_input = MagicMock(spec = Element)
|
||||
@@ -3057,7 +3040,7 @@ class TestCategorySuggestionPicker:
|
||||
class TestShippingDialogFlow:
|
||||
"""Regression tests for shipping dialog flow using new radio selectors only."""
|
||||
|
||||
shipping_combobox_selector = _VERSAND_COMBOBOX_SELECTOR # noqa: SLF001 - intentional single source of truth for selector tests
|
||||
shipping_combobox_selector = VERSAND_COMBOBOX_SELECTOR
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
@@ -3557,7 +3540,7 @@ class TestWantedShippingSelection:
|
||||
dispatch happen during ``publish_ad``.
|
||||
"""
|
||||
|
||||
shipping_combobox_selector = _VERSAND_COMBOBOX_SELECTOR # noqa: SLF001 - intentional single source of truth for selector tests
|
||||
shipping_combobox_selector = VERSAND_COMBOBOX_SELECTOR
|
||||
|
||||
@contextmanager
|
||||
def _mock_publish_dependencies(
|
||||
|
||||
Reference in New Issue
Block a user