fix: publishing contact fields and download stability #771
+126
−57
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #771 +/- ##
=======================================
Coverage 88.58% 88.58%
=======================================
Files 20 20
Lines 2172 2173 +1
Branches 423 423
=======================================
+ Hits 1924 1925 +1
Misses 120 120
Partials 128 128
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughCentralizes contact-field handling in KleinanzeigenBot by adding a private async method Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/kleinanzeigen_bot/__init__.py`:
- Around line 1129-1133: The wait is using self.web_sleep(1) but web_sleep
expects milliseconds, so it's only pausing ~1ms and the city dropdown won't
populate; update the call in the block that checks contact.location to use a
realistic millisecond delay (e.g., self.web_sleep(1000)) or replace with a short
retry loop that calls web_find_all(By.CSS_SELECTOR, "#pstad-citychsr option")
until options length is >1 with small web_sleep intervals to robustly wait for
the dropdown to populate.
- Around line 1149-1163: The contact name/street DOM interactions can raise
TimeoutError and currently may abort publishing; wrap the calls around
web_check/web_click/web_sleep/web_input for the "pstad-street" logic and the
web_check/web_input for "postad-contactname" in try/except blocks that catch
TimeoutError and either log/skip the field or re-raise with contextual message;
specifically modify the section that uses contact.street, web_check(By.ID,
"pstad-street", Is.DISABLED), web_click("addressVisibility"), web_sleep(),
web_input("pstad-street") and the section that checks postad-contactname
(web_check(By.ID, "postad-contactname", Is.READONLY) and
web_input("postad-contactname")) to handle TimeoutError consistently (e.g., log
a warning including which field failed and continue publishing or raise a new
TimeoutError with added context).
- Around line 1559-1564: The debug message logged when skipping a saved ad
without an id (inside the loop over ads where LOG.debug("Skipping saved ad
without id: %s", ad[0]) is called) must be localized; replace the literal string
with a call to the project's translation helper (e.g. _("Skipping saved ad
without id: %s") or gettext.gettext(...)) and ensure the translation function is
imported/available in this module so the LOG.debug call uses the translated
string while keeping the placeholder and ad[0] argument intact; keep the
surrounding logic (saved_ad_id check, continue, saved_ad_ids.append) unchanged.
In `@src/kleinanzeigen_bot/model/config_model.py`:
- Around line 66-70: The new Pydantic field "location" in config_model.py lacks
Field metadata; update the "location: str | None = None" declaration to include
a Field(...) with a concise description and an example (e.g., description="City
or locality of the listing", example="Berlin") to keep schema docs consistent,
and ensure Field is imported from pydantic at the top if not already present;
modify the declaration in the class where "name", "street", "zipcode",
"location", "phone" are defined (look for the attribute named location) to use
Field for the default instead of a bare None.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/kleinanzeigen_bot/__init__.py (1)
15-26: Use absolute imports for project modules.The new Contact import keeps the relative style; project guidelines require absolute imports for project modules. Consider switching this local import block to absolute paths for consistency.
♻️ Proposed update
-from . import extract, resources -from ._version import __version__ -from .model.ad_model import MAX_DESCRIPTION_LENGTH, Ad, AdPartial, Contact, calculate_auto_price -from .model.config_model import Config -from .update_checker import UpdateChecker -from .utils import dicts, error_handlers, loggers, misc -from .utils.exceptions import CaptchaEncountered -from .utils.files import abspath -from .utils.i18n import Locale, get_current_locale, pluralize, set_current_locale -from .utils.misc import ainput, ensure, is_frozen -from .utils.web_scraping_mixin import By, Element, Is, WebScrapingMixin +from kleinanzeigen_bot import extract, resources +from kleinanzeigen_bot._version import __version__ +from kleinanzeigen_bot.model.ad_model import MAX_DESCRIPTION_LENGTH, Ad, AdPartial, Contact, calculate_auto_price +from kleinanzeigen_bot.model.config_model import Config +from kleinanzeigen_bot.update_checker import UpdateChecker +from kleinanzeigen_bot.utils import dicts, error_handlers, loggers, misc +from kleinanzeigen_bot.utils.exceptions import CaptchaEncountered +from kleinanzeigen_bot.utils.files import abspath +from kleinanzeigen_bot.utils.i18n import Locale, get_current_locale, pluralize, set_current_locale +from kleinanzeigen_bot.utils.misc import ainput, ensure, is_frozen +from kleinanzeigen_bot.utils.web_scraping_mixin import By, Element, Is, WebScrapingMixinAs per coding guidelines, please use absolute imports for project modules.
🤖 Fix all issues with AI agents
In `@src/kleinanzeigen_bot/__init__.py`:
- Around line 1119-1127: The ZIP input sequence can raise a TimeoutError from
web_input and currently would abort publishing; update the block that handles
contact.zipcode (using web_find and web_input for By.ID "pstad-zip") to catch
TimeoutError around the web_input call, log a concise warning (including contact
identifier/context) and skip continuing ZIP entry instead of letting the
exception bubble; keep the existing fallback for clear_input but ensure both the
clear and the subsequent web_input are guarded so any TimeoutError is caught and
handled gracefully.
♻️ Duplicate comments (2)
src/kleinanzeigen_bot/__init__.py (2)
1129-1133: Increase web_sleep delay for city dropdown.Already flagged in a prior review:
web_sleepexpects milliseconds;1is ~1ms and too short for the dropdown to populate.
1149-1163: Guard street/name web operations with TimeoutError handling.Already flagged in a prior review; missing handling can abort publishing on DOM variants.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/kleinanzeigen_bot/__init__.py`:
- Around line 1134-1148: The current loop in the contact location selection
(inside the block using self.web_find_all and self.web_select) swallows the case
where no option matches contact.location; add a boolean flag (e.g., found =
False) before iterating options, set found = True when you call self.web_select,
and after the loop (still inside the try) if found is False emit a LOG.warning
(or LOG.info) noting that no dropdown option matched contact.location so the
location was ignored; keep the existing TimeoutError except block unchanged.
- Around line 1121-1127: The guard checking for None after calling self.web_find
is redundant because web_find either returns an Element or raises TimeoutError;
update the try block in the initialization flow to call await
zip_field.clear_input() directly after zip_field = await self.web_find(By.ID,
"pstad-zip") and remove the if zip_field is not None: conditional, keeping the
existing TimeoutError except block intact; refer to the zip_field variable and
the self.web_find(By.ID, "pstad-zip") call to locate the change.
sebthom
previously approved these changes
Jan 18, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/kleinanzeigen_bot/__init__.py`:
- Line 17: Replace the relative import in __init__.py with an absolute import:
update the statement that currently imports MAX_DESCRIPTION_LENGTH, Ad,
AdPartial, Contact, calculate_auto_price from .model.ad_model to import them
using the package name (e.g., kleinanzeigen_bot.model.ad_model) so the symbols
MAX_DESCRIPTION_LENGTH, Ad, AdPartial, Contact, and calculate_auto_price are
imported via an absolute path.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ℹ️ Description
📋 Changes Summary
download --ads=newwhen existing local ads lack an ID; skip those files for the “already downloaded” set and log a clear reason.-.contact.locationin defaults (config model + regenerated schema with example).⚙️ Type of Change
Select the type(s) of change(s) included in this pull request:
✅ Checklist
Before requesting a review, confirm the following:
pdm run test).pdm run format).pdm run lint).By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.