mirror of
https://github.com/Second-Hand-Friends/kleinanzeigen-bot.git
synced 2026-08-24 07:41:45 +02:00
fix: handle idless update confirmations (#1244)
## ℹ️ Description - Related issue: #1228 - Handles the redesigned ID-less success page when an existing listing is updated. ## 📋 Changes Summary - Recognize the existing ID-less success-page marker for updates as well as publishes. - Reuse the configured listing ID for a confirmed update; do not attempt publish-style new-ID recovery. - Preserve the strict published-ads recovery path for replacement publishes. - Add German translation coverage and a focused unit test. ### ⚙️ 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 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] Documentation changes are not needed for this internal recovery-path fix. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved publishing and updating reliability for redesigned confirmation pages that do not display an ad ID. * Updates now use the configured ad ID when available and report uncertainty when it is missing. * New listings recover their ad ID from published listings and report uncertain outcomes when recovery fails or is ambiguous. * Added German messaging for updates confirmed without a displayed ad ID. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Jens
GitHub
@@ -252,7 +252,7 @@ async def submit_and_confirm_ad(
|
||||
url = str(await web.web_execute("window.location.href"))
|
||||
if "p-anzeige-aufgeben-bestaetigung.html?adId=" in url:
|
||||
return True
|
||||
if mode == AdUpdateStrategy.REPLACE and await _is_idless_publish_success_page(web):
|
||||
if await _is_idless_publish_success_page(web):
|
||||
idless_success_detected = True
|
||||
return True
|
||||
return False
|
||||
@@ -260,26 +260,37 @@ async def submit_and_confirm_ad(
|
||||
await web.web_await(_check_confirmation_state, timeout = confirmation_timeout)
|
||||
|
||||
if idless_success_detected:
|
||||
try:
|
||||
ad_id = await _try_recover_ad_id_from_published_ads(
|
||||
web,
|
||||
root_url = root_url,
|
||||
title = ad_cfg.title,
|
||||
known_published_ad_ids = known_published_ad_ids,
|
||||
if mode == AdUpdateStrategy.MODIFY:
|
||||
ad_id = ad_cfg.id
|
||||
if ad_id is None:
|
||||
raise PublishSubmissionUncertainError(
|
||||
_("update succeeded but the configured ad ID is missing")
|
||||
)
|
||||
LOG.warning(
|
||||
"Update confirmation page exposed no ad ID; using configured ad ID %s",
|
||||
ad_id,
|
||||
)
|
||||
except Exception as recovery_ex: # noqa: BLE001
|
||||
LOG.debug("Published-ad list fallback failed: %s", recovery_ex)
|
||||
raise PublishSubmissionUncertainError(
|
||||
"publish succeeded but no ad ID could be recovered"
|
||||
) from recovery_ex
|
||||
if ad_id is None:
|
||||
raise PublishSubmissionUncertainError(
|
||||
"publish succeeded but no ad ID could be recovered"
|
||||
else:
|
||||
try:
|
||||
ad_id = await _try_recover_ad_id_from_published_ads(
|
||||
web,
|
||||
root_url = root_url,
|
||||
title = ad_cfg.title,
|
||||
known_published_ad_ids = known_published_ad_ids,
|
||||
)
|
||||
except Exception as recovery_ex: # noqa: BLE001
|
||||
LOG.debug("Published-ad list fallback failed: %s", recovery_ex)
|
||||
raise PublishSubmissionUncertainError(
|
||||
"publish succeeded but no ad ID could be recovered"
|
||||
) from recovery_ex
|
||||
if ad_id is None:
|
||||
raise PublishSubmissionUncertainError(
|
||||
"publish succeeded but no ad ID could be recovered"
|
||||
)
|
||||
LOG.warning(
|
||||
"Confirmation page exposed no ad ID; recovered ad ID %s from the published ads list",
|
||||
ad_id,
|
||||
)
|
||||
LOG.warning(
|
||||
"Confirmation page exposed no ad ID; recovered ad ID %s from the published ads list",
|
||||
ad_id,
|
||||
)
|
||||
else:
|
||||
# Use the live URL because the page object URL may be stale after redirects.
|
||||
current_url = str(await web.web_execute("window.location.href"))
|
||||
|
||||
@@ -386,6 +386,8 @@ kleinanzeigen_bot/publishing_submission.py:
|
||||
"Press a key to continue...": "Eine Taste drücken, um fortzufahren..."
|
||||
"Confirmation page redirected too fast; extracted ad ID %s from page tracking data": "Bestätigungsseite wurde zu schnell weitergeleitet; Anzeigen-ID %s aus Seiten-Trackingdaten extrahiert"
|
||||
"Confirmation page exposed no ad ID; recovered ad ID %s from the published ads list": "Bestätigungsseite enthielt keine Anzeigen-ID; Anzeigen-ID %s aus der Liste veröffentlichter Anzeigen wiederhergestellt"
|
||||
"Update confirmation page exposed no ad ID; using configured ad ID %s": "Update-Bestätigungsseite enthielt keine Anzeigen-ID; konfigurierte Anzeigen-ID %s wird verwendet"
|
||||
"update succeeded but the configured ad ID is missing": "Aktualisierung war erfolgreich, aber die konfigurierte Anzeigen-ID fehlt"
|
||||
"ad_id is unexpectedly None after confirmation flow for %s": "ad_id ist unerwartet None nach dem Bestätigungsablauf für %s"
|
||||
|
||||
#################################################
|
||||
|
||||
@@ -452,6 +452,160 @@ class TestPublishedAdsRecovery:
|
||||
known_published_ad_ids = frozenset({10}),
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_submit_accepts_idless_success_after_update(self, test_bot:KleinanzeigenBot) -> None:
|
||||
"""An ID-less update confirmation reuses the configured ad ID."""
|
||||
ad = _make_min_ad()
|
||||
ad.id = 777
|
||||
|
||||
async def await_condition(condition:Any, **_:object) -> bool:
|
||||
return bool(await condition())
|
||||
|
||||
with (
|
||||
patch("kleinanzeigen_bot.captcha_flow.check_and_wait_for_captcha", new_callable = AsyncMock),
|
||||
patch.object(test_bot, "web_set_input_value", new_callable = AsyncMock),
|
||||
patch.object(test_bot, "web_click", new_callable = AsyncMock),
|
||||
patch.object(test_bot, "web_probe", new_callable = AsyncMock, side_effect = [None] * 4),
|
||||
patch.object(test_bot, "web_await", new_callable = AsyncMock, side_effect = await_condition),
|
||||
patch.object(
|
||||
test_bot,
|
||||
"web_execute",
|
||||
new_callable = AsyncMock,
|
||||
side_effect = ["", f"{test_bot.root_url}/done"],
|
||||
),
|
||||
patch(
|
||||
"kleinanzeigen_bot.publishing_submission._is_idless_publish_success_page",
|
||||
new_callable = AsyncMock,
|
||||
return_value = True,
|
||||
),
|
||||
patch(
|
||||
"kleinanzeigen_bot.publishing_submission._try_recover_ad_id_from_published_ads",
|
||||
new_callable = AsyncMock,
|
||||
) as recover_mock,
|
||||
):
|
||||
result = await publishing_submission.submit_and_confirm_ad(
|
||||
test_bot,
|
||||
"test.yaml",
|
||||
ad,
|
||||
AdUpdateStrategy.MODIFY,
|
||||
captcha_config = test_bot.config.captcha,
|
||||
root_url = test_bot.root_url,
|
||||
)
|
||||
|
||||
assert result == 777
|
||||
recover_mock.assert_not_awaited()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_submit_rejects_idless_update_success_without_configured_id(self, test_bot:KleinanzeigenBot) -> None:
|
||||
"""An update cannot recover safely if its configured ID is unexpectedly absent."""
|
||||
ad = _make_min_ad()
|
||||
|
||||
async def await_condition(condition:Any, **_:object) -> bool:
|
||||
return bool(await condition())
|
||||
|
||||
with (
|
||||
patch("kleinanzeigen_bot.captcha_flow.check_and_wait_for_captcha", new_callable = AsyncMock),
|
||||
patch.object(test_bot, "web_set_input_value", new_callable = AsyncMock),
|
||||
patch.object(test_bot, "web_click", new_callable = AsyncMock),
|
||||
patch.object(test_bot, "web_probe", new_callable = AsyncMock, side_effect = [None] * 4),
|
||||
patch.object(test_bot, "web_await", new_callable = AsyncMock, side_effect = await_condition),
|
||||
patch.object(test_bot, "web_execute", new_callable = AsyncMock, side_effect = ["", f"{test_bot.root_url}/done"]),
|
||||
patch(
|
||||
"kleinanzeigen_bot.publishing_submission._is_idless_publish_success_page",
|
||||
new_callable = AsyncMock,
|
||||
return_value = True,
|
||||
),
|
||||
pytest.raises(PublishSubmissionUncertainError, match = "configured ad ID is missing"),
|
||||
):
|
||||
await publishing_submission.submit_and_confirm_ad(
|
||||
test_bot,
|
||||
"test.yaml",
|
||||
ad,
|
||||
AdUpdateStrategy.MODIFY,
|
||||
captcha_config = test_bot.config.captcha,
|
||||
root_url = test_bot.root_url,
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_submit_falls_back_to_tracking_when_idless_marker_is_absent(self, test_bot:KleinanzeigenBot) -> None:
|
||||
"""An unrecognized confirmation page retains the existing tracking fallback."""
|
||||
ad = _make_min_ad()
|
||||
|
||||
async def await_condition(condition:Any, **_:object) -> bool:
|
||||
return bool(await condition())
|
||||
|
||||
with (
|
||||
patch("kleinanzeigen_bot.captcha_flow.check_and_wait_for_captcha", new_callable = AsyncMock),
|
||||
patch.object(test_bot, "web_set_input_value", new_callable = AsyncMock),
|
||||
patch.object(test_bot, "web_click", new_callable = AsyncMock),
|
||||
patch.object(test_bot, "web_probe", new_callable = AsyncMock, side_effect = [None] * 4),
|
||||
patch.object(test_bot, "web_await", new_callable = AsyncMock, side_effect = await_condition),
|
||||
patch.object(
|
||||
test_bot,
|
||||
"web_execute",
|
||||
new_callable = AsyncMock,
|
||||
side_effect = ["", f"{test_bot.root_url}/done", f"{test_bot.root_url}/done"],
|
||||
),
|
||||
patch(
|
||||
"kleinanzeigen_bot.publishing_submission._is_idless_publish_success_page",
|
||||
new_callable = AsyncMock,
|
||||
return_value = False,
|
||||
),
|
||||
patch(
|
||||
"kleinanzeigen_bot.publishing_submission._try_recover_ad_id_from_redirect",
|
||||
new_callable = AsyncMock,
|
||||
return_value = 777,
|
||||
) as tracking_recover_mock,
|
||||
):
|
||||
result = await publishing_submission.submit_and_confirm_ad(
|
||||
test_bot,
|
||||
"test.yaml",
|
||||
ad,
|
||||
AdUpdateStrategy.MODIFY,
|
||||
captcha_config = test_bot.config.captcha,
|
||||
root_url = test_bot.root_url,
|
||||
)
|
||||
|
||||
assert result == 777
|
||||
tracking_recover_mock.assert_awaited_once_with(test_bot, pre_submit_referrer = "")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_submit_fails_closed_when_idless_publish_recovery_finds_no_ad(self, test_bot:KleinanzeigenBot) -> None:
|
||||
"""An ID-less publish confirmation remains uncertain without one new exact-title ad."""
|
||||
ad = _make_min_ad()
|
||||
|
||||
async def await_condition(condition:Any, **_:object) -> bool:
|
||||
return bool(await condition())
|
||||
|
||||
with (
|
||||
patch("kleinanzeigen_bot.captcha_flow.check_and_wait_for_captcha", new_callable = AsyncMock),
|
||||
patch.object(test_bot, "web_set_input_value", new_callable = AsyncMock),
|
||||
patch.object(test_bot, "web_click", new_callable = AsyncMock),
|
||||
patch.object(test_bot, "web_probe", new_callable = AsyncMock, side_effect = [None] * 4),
|
||||
patch.object(test_bot, "web_await", new_callable = AsyncMock, side_effect = await_condition),
|
||||
patch.object(test_bot, "web_execute", new_callable = AsyncMock, side_effect = ["", f"{test_bot.root_url}/done"]),
|
||||
patch(
|
||||
"kleinanzeigen_bot.publishing_submission._is_idless_publish_success_page",
|
||||
new_callable = AsyncMock,
|
||||
return_value = True,
|
||||
),
|
||||
patch(
|
||||
"kleinanzeigen_bot.publishing_submission._try_recover_ad_id_from_published_ads",
|
||||
new_callable = AsyncMock,
|
||||
return_value = None,
|
||||
),
|
||||
pytest.raises(PublishSubmissionUncertainError, match = "no ad ID could be recovered"),
|
||||
):
|
||||
await publishing_submission.submit_and_confirm_ad(
|
||||
test_bot,
|
||||
"test.yaml",
|
||||
ad,
|
||||
AdUpdateStrategy.REPLACE,
|
||||
captcha_config = test_bot.config.captcha,
|
||||
root_url = test_bot.root_url,
|
||||
known_published_ad_ids = frozenset({10}),
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_submit_fails_closed_when_published_ads_recovery_raises(
|
||||
self,
|
||||
|
||||
Reference in New Issue
Block a user