mirror of
https://github.com/Second-Hand-Friends/kleinanzeigen-bot.git
synced 2026-07-09 12:41:05 +02:00
fix(update): include ads with pending auto price reductions in --ads changed (#1067)
## ℹ️ Description `pdm run app update` (default `--ads changed`) loads 0 ads even when `verify` shows pending price reductions. Root cause: `__check_ad_changed()` only compares `content_hash`, but auto price reduction is time-driven. - Link to the related issue(s): Issue #1064 ## 📋 Changes Summary - **`price_reduction.py`**: Added `is_auto_price_reduction_due(ad_cfg, ad_file_relative) -> bool` — evaluates price reduction in MODIFY mode and returns true when a visible reduction cycle is due - **`__init__.py` — `load_ads()`**: The `"changed"` selector now also checks `is_auto_price_reduction_due()` **only during `update` command** via `self.command` guard — publish is unaffected - **`translations.de.yaml`**: Added German translation under `price_reduction.py:is_auto_price_reduction_due` - **Tests**: 8 new unit tests for `is_auto_price_reduction_due()` (5 mocked + 3 real eligibility with timing) in `test_price_reduction.py`; 3 integration tests in `test_init.py` including regression test for publish mode exclusion ### ⚙️ Type of Change - [x] 🐞 Bug fix (non-breaking change which fixes an issue) ## ✅ 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. Closes #1064 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Automatic price reductions are now considered when identifying changed ads during updates. Ads with pending price reductions are included in the "changed" selection for update operations, regardless of content modifications. * **Tests** * Added comprehensive test coverage for automatic price reduction due-state detection and update behavior validation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -54,6 +54,9 @@ from .price_reduction import (
|
||||
from .price_reduction import (
|
||||
evaluate_auto_price_reduction as evaluate_auto_price_reduction,
|
||||
)
|
||||
from .price_reduction import (
|
||||
is_auto_price_reduction_due as is_auto_price_reduction_due,
|
||||
)
|
||||
from .update_checker import UpdateChecker
|
||||
from .utils import diagnostics, dicts, error_handlers, loggers, misc, xdg_paths
|
||||
from .utils.exceptions import CaptchaEncountered, CategoryResolutionError, PublishedAdsFetchIncompleteError, PublishSubmissionUncertainError
|
||||
@@ -1084,6 +1087,8 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
|
||||
# Check for 'changed' selector
|
||||
if "changed" in selectors and self.__check_ad_changed(ad_cfg, ad_cfg_orig, ad_file_relative):
|
||||
should_include = True
|
||||
elif "changed" in selectors and self.command == "update" and is_auto_price_reduction_due(ad_cfg, ad_file_relative):
|
||||
should_include = True
|
||||
|
||||
# Check for 'new' selector
|
||||
if "new" in selectors and (not ad_cfg.id or not exclude_ads_with_id):
|
||||
|
||||
@@ -9,6 +9,7 @@ __all__ = [
|
||||
"_log_auto_price_reduction_preview",
|
||||
"apply_auto_price_reduction",
|
||||
"evaluate_auto_price_reduction",
|
||||
"is_auto_price_reduction_due",
|
||||
]
|
||||
|
||||
from dataclasses import dataclass
|
||||
@@ -297,6 +298,45 @@ def evaluate_auto_price_reduction(ad_cfg:Ad, _ad_file_relative:str, *, mode:AdUp
|
||||
)
|
||||
|
||||
|
||||
def is_auto_price_reduction_due(ad_cfg:Ad, ad_file_relative:str) -> bool:
|
||||
"""Check if an ad has a pending auto price reduction that should trigger an update.
|
||||
|
||||
Evaluates the auto price reduction in MODIFY mode without mutating the ad.
|
||||
A reduction is 'due' when it is enabled, the delay criteria are satisfied,
|
||||
a next cycle is calculated, and the result price differs from the restored price.
|
||||
|
||||
Note:
|
||||
``no_visible_change`` reductions (where the calculated price does not
|
||||
change, but ``price_reduction_count`` would still advance) are not
|
||||
considered 'due' here, since only visible price changes warrant a
|
||||
browser update.
|
||||
|
||||
Args:
|
||||
ad_cfg: The ad configuration to evaluate.
|
||||
ad_file_relative: Relative path to the ad file (for logging context).
|
||||
|
||||
Returns:
|
||||
True if a price reduction is due for this ad.
|
||||
"""
|
||||
if not ad_cfg.id:
|
||||
# New ads don't have pending reductions
|
||||
return False
|
||||
|
||||
decision = evaluate_auto_price_reduction(ad_cfg, ad_file_relative, mode = AdUpdateStrategy.MODIFY)
|
||||
|
||||
if not decision.enabled:
|
||||
return False
|
||||
|
||||
if decision.next_cycle is None:
|
||||
return False
|
||||
|
||||
if not decision.cycle_advanced:
|
||||
return False
|
||||
|
||||
LOG.info("Pending auto price reduction detected for ad [%s]", ad_file_relative)
|
||||
return True
|
||||
|
||||
|
||||
def _log_auto_price_reduction_preview(ad_file_relative:str, decision:PriceReductionDecision) -> None:
|
||||
mode_label = _("publish") if decision.mode == AdUpdateStrategy.REPLACE else _("update")
|
||||
if not decision.enabled:
|
||||
|
||||
@@ -374,6 +374,9 @@ kleinanzeigen_bot/__init__.py:
|
||||
#################################################
|
||||
kleinanzeigen_bot/price_reduction.py:
|
||||
#################################################
|
||||
is_auto_price_reduction_due:
|
||||
"Pending auto price reduction detected for ad [%s]": "Ausstehende automatische Preisreduzierung für Anzeige [%s] erkannt"
|
||||
|
||||
_log_auto_price_reduction_preview:
|
||||
"publish": "veröffentlichen"
|
||||
"update": "aktualisieren"
|
||||
|
||||
@@ -5485,6 +5485,178 @@ class TestKleinanzeigenBotChangedAds:
|
||||
# The changed ad should be loaded with 'due' selector because it's due for republication
|
||||
assert len(ads_to_publish) == 1
|
||||
|
||||
def test_load_ads_with_changed_selector_and_pending_price_reduction(self, test_bot_config:Config, base_ad_config:dict[str, Any]) -> None:
|
||||
"""Test that 'changed' selector also loads ads with pending auto price reductions."""
|
||||
test_bot = KleinanzeigenBot()
|
||||
test_bot.ads_selector = "changed"
|
||||
test_bot.command = "update"
|
||||
test_bot.config = test_bot_config.with_values({"ad_defaults": {"description": {"prefix": "", "suffix": ""}}})
|
||||
|
||||
# Create an ad with auto_price_reduction configured and ready to trigger
|
||||
ad_cfg = Ad.model_validate(
|
||||
base_ad_config
|
||||
| {
|
||||
"id": "12345",
|
||||
"title": "Ad With Price Reduction",
|
||||
"updated_on": "2024-01-01T00:00:00",
|
||||
"created_on": "2024-01-01T00:00:00",
|
||||
"price": 100,
|
||||
"price_reduction_count": 0,
|
||||
"repost_count": 1,
|
||||
"active": True,
|
||||
"auto_price_reduction": {
|
||||
"enabled": True,
|
||||
"on_update": True,
|
||||
"strategy": "FIXED",
|
||||
"amount": 10,
|
||||
"min_price": 1,
|
||||
"delay_days": 0,
|
||||
"delay_reposts": 0,
|
||||
},
|
||||
}
|
||||
)
|
||||
|
||||
# Store the content hash so __check_ad_changed sees no change
|
||||
ad_dict = ad_cfg.model_dump()
|
||||
ad_dict["content_hash"] = ad_cfg.update_content_hash().content_hash
|
||||
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
temp_path = Path(temp_dir)
|
||||
ad_dir = temp_path / "ads"
|
||||
ad_dir.mkdir()
|
||||
|
||||
dicts.save_dict(ad_dir / "ad_with_reduction.yaml", ad_dict)
|
||||
|
||||
test_bot.config_file_path = str(temp_path / "config.yaml")
|
||||
test_bot.config.ad_files = ["ads/*.yaml"]
|
||||
|
||||
with patch(
|
||||
"kleinanzeigen_bot.utils.dicts.load_dict",
|
||||
side_effect = [
|
||||
ad_dict, # First call returns the ad
|
||||
{}, # Second call for ad_fields.yaml
|
||||
],
|
||||
):
|
||||
ads_to_publish = test_bot.load_ads()
|
||||
|
||||
# The ad should be loaded because a price reduction is pending
|
||||
assert len(ads_to_publish) == 1
|
||||
assert ads_to_publish[0][1].title == "Ad With Price Reduction"
|
||||
|
||||
def test_load_ads_with_changed_selector_no_price_reduction_when_not_configured(self, test_bot_config:Config, base_ad_config:dict[str, Any]) -> None:
|
||||
"""Test that 'changed' selector does not load an unchanged ad when auto_price_reduction is disabled."""
|
||||
test_bot = KleinanzeigenBot()
|
||||
test_bot.ads_selector = "changed"
|
||||
test_bot.command = "update"
|
||||
test_bot.config = test_bot_config.with_values({"ad_defaults": {"description": {"prefix": "", "suffix": ""}}})
|
||||
|
||||
# Create an ad with auto_price_reduction disabled
|
||||
ad_cfg = Ad.model_validate(
|
||||
base_ad_config
|
||||
| {
|
||||
"id": "12345",
|
||||
"title": "Unchanged Ad",
|
||||
"updated_on": "2024-01-01T00:00:00",
|
||||
"created_on": "2024-01-01T00:00:00",
|
||||
"price": 100,
|
||||
"repost_count": 1,
|
||||
"active": True,
|
||||
"auto_price_reduction": {
|
||||
"enabled": False,
|
||||
"on_update": True,
|
||||
"strategy": "FIXED",
|
||||
"amount": 10,
|
||||
"min_price": 1,
|
||||
"delay_days": 0,
|
||||
"delay_reposts": 0,
|
||||
},
|
||||
}
|
||||
)
|
||||
|
||||
# Store the content hash so __check_ad_changed sees no change
|
||||
ad_dict = ad_cfg.model_dump()
|
||||
ad_dict["content_hash"] = ad_cfg.update_content_hash().content_hash
|
||||
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
temp_path = Path(temp_dir)
|
||||
ad_dir = temp_path / "ads"
|
||||
ad_dir.mkdir()
|
||||
|
||||
dicts.save_dict(ad_dir / "unchanged_ad.yaml", ad_dict)
|
||||
|
||||
test_bot.config_file_path = str(temp_path / "config.yaml")
|
||||
test_bot.config.ad_files = ["ads/*.yaml"]
|
||||
|
||||
with patch(
|
||||
"kleinanzeigen_bot.utils.dicts.load_dict",
|
||||
side_effect = [
|
||||
ad_dict, # First call returns the ad
|
||||
{}, # Second call for ad_fields.yaml
|
||||
],
|
||||
):
|
||||
ads_to_publish = test_bot.load_ads()
|
||||
|
||||
# The ad should NOT be loaded because it's unchanged and auto_price_reduction is disabled
|
||||
assert len(ads_to_publish) == 0
|
||||
|
||||
def test_load_ads_with_changed_selector_does_not_include_price_reduction_in_publish_mode(
|
||||
self, test_bot_config:Config, base_ad_config:dict[str, Any]) -> None:
|
||||
"""Test that 'changed' selector in publish mode skips unchanged ads even when price reduction is pending."""
|
||||
test_bot = KleinanzeigenBot()
|
||||
test_bot.ads_selector = "changed"
|
||||
test_bot.command = "publish"
|
||||
test_bot.config = test_bot_config.with_values({"ad_defaults": {"description": {"prefix": "", "suffix": ""}}})
|
||||
|
||||
# Create an ad with auto_price_reduction configured and ready to trigger
|
||||
ad_cfg = Ad.model_validate(
|
||||
base_ad_config
|
||||
| {
|
||||
"id": "12345",
|
||||
"title": "Ad With Price Reduction",
|
||||
"updated_on": "2024-01-01T00:00:00",
|
||||
"created_on": "2024-01-01T00:00:00",
|
||||
"price": 100,
|
||||
"price_reduction_count": 0,
|
||||
"repost_count": 1,
|
||||
"active": True,
|
||||
"auto_price_reduction": {
|
||||
"enabled": True,
|
||||
"on_update": True,
|
||||
"strategy": "FIXED",
|
||||
"amount": 10,
|
||||
"min_price": 1,
|
||||
"delay_days": 0,
|
||||
"delay_reposts": 0,
|
||||
},
|
||||
}
|
||||
)
|
||||
|
||||
# Store the content hash so __check_ad_changed sees no change
|
||||
ad_dict = ad_cfg.model_dump()
|
||||
ad_dict["content_hash"] = ad_cfg.update_content_hash().content_hash
|
||||
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
temp_path = Path(temp_dir)
|
||||
ad_dir = temp_path / "ads"
|
||||
ad_dir.mkdir()
|
||||
|
||||
dicts.save_dict(ad_dir / "ad_with_reduction.yaml", ad_dict)
|
||||
|
||||
test_bot.config_file_path = str(temp_path / "config.yaml")
|
||||
test_bot.config.ad_files = ["ads/*.yaml"]
|
||||
|
||||
with patch(
|
||||
"kleinanzeigen_bot.utils.dicts.load_dict",
|
||||
side_effect = [
|
||||
ad_dict, # First call returns the ad
|
||||
{}, # Second call for ad_fields.yaml
|
||||
],
|
||||
):
|
||||
ads_to_publish = test_bot.load_ads()
|
||||
|
||||
# The ad should NOT be loaded because price reduction is only applied in update mode
|
||||
assert len(ads_to_publish) == 0
|
||||
|
||||
|
||||
def test_file_logger_writes_message(tmp_path:Path, caplog:pytest.LogCaptureFixture) -> None:
|
||||
"""
|
||||
|
||||
@@ -777,6 +777,192 @@ def test_apply_with_null_config_does_not_crash(
|
||||
assert ad_cfg.price == 100 # unchanged, no reduction to apply
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_is_auto_price_reduction_due_is_false_when_no_ad_id() -> None:
|
||||
"""is_auto_price_reduction_due returns False when ad has no id (new ad)."""
|
||||
ad_cfg = SimpleNamespace(
|
||||
id = None,
|
||||
price = 100,
|
||||
auto_price_reduction = None,
|
||||
price_reduction_count = 0,
|
||||
repost_count = 0,
|
||||
updated_on = None,
|
||||
created_on = None,
|
||||
)
|
||||
assert kleinanzeigen_bot.is_auto_price_reduction_due(ad_cfg, "ad_new.yaml") is False # type: ignore[arg-type]
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_is_auto_price_reduction_due_is_false_when_not_enabled(
|
||||
monkeypatch:pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""is_auto_price_reduction_due returns False when auto_price_reduction is not enabled."""
|
||||
ad_cfg = SimpleNamespace(
|
||||
id = 12345,
|
||||
price = 100,
|
||||
auto_price_reduction = None,
|
||||
price_reduction_count = 0,
|
||||
repost_count = 0,
|
||||
updated_on = None,
|
||||
created_on = None,
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"kleinanzeigen_bot.price_reduction.evaluate_auto_price_reduction",
|
||||
lambda *args, **kwargs: SimpleNamespace(
|
||||
enabled = False,
|
||||
next_cycle = 1,
|
||||
cycle_advanced = True,
|
||||
),
|
||||
)
|
||||
|
||||
assert kleinanzeigen_bot.is_auto_price_reduction_due(ad_cfg, "ad_disabled.yaml") is False # type: ignore[arg-type]
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_is_auto_price_reduction_due_is_false_when_next_cycle_is_none(
|
||||
monkeypatch:pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""is_auto_price_reduction_due returns False when next_cycle is None."""
|
||||
ad_cfg = SimpleNamespace(
|
||||
id = 12345,
|
||||
price = 100,
|
||||
auto_price_reduction = None,
|
||||
price_reduction_count = 0,
|
||||
repost_count = 0,
|
||||
updated_on = None,
|
||||
created_on = None,
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"kleinanzeigen_bot.price_reduction.evaluate_auto_price_reduction",
|
||||
lambda *args, **kwargs: SimpleNamespace(
|
||||
enabled = True,
|
||||
next_cycle = None,
|
||||
cycle_advanced = True,
|
||||
),
|
||||
)
|
||||
|
||||
assert kleinanzeigen_bot.is_auto_price_reduction_due(ad_cfg, "ad_no_next_cycle.yaml") is False # type: ignore[arg-type]
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_is_auto_price_reduction_due_is_false_when_cycle_not_advanced(
|
||||
monkeypatch:pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""is_auto_price_reduction_due returns False when cycle_advanced is False."""
|
||||
ad_cfg = SimpleNamespace(
|
||||
id = 12345,
|
||||
price = 100,
|
||||
auto_price_reduction = None,
|
||||
price_reduction_count = 0,
|
||||
repost_count = 0,
|
||||
updated_on = None,
|
||||
created_on = None,
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"kleinanzeigen_bot.price_reduction.evaluate_auto_price_reduction",
|
||||
lambda *args, **kwargs: SimpleNamespace(
|
||||
enabled = True,
|
||||
next_cycle = 1,
|
||||
cycle_advanced = False,
|
||||
),
|
||||
)
|
||||
|
||||
assert kleinanzeigen_bot.is_auto_price_reduction_due(ad_cfg, "ad_no_cycle_advance.yaml") is False # type: ignore[arg-type]
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_is_auto_price_reduction_due_returns_true_when_reduction_due(
|
||||
monkeypatch:pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""is_auto_price_reduction_due returns True when a reduction is actually due (enabled, next_cycle set, cycle_advanced)."""
|
||||
ad_cfg = SimpleNamespace(
|
||||
id = 12345,
|
||||
price = 100,
|
||||
auto_price_reduction = None,
|
||||
price_reduction_count = 0,
|
||||
repost_count = 0,
|
||||
updated_on = None,
|
||||
created_on = None,
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"kleinanzeigen_bot.price_reduction.evaluate_auto_price_reduction",
|
||||
lambda *args, **kwargs: SimpleNamespace(
|
||||
enabled = True,
|
||||
next_cycle = 1,
|
||||
cycle_advanced = True,
|
||||
),
|
||||
)
|
||||
|
||||
assert kleinanzeigen_bot.is_auto_price_reduction_due(ad_cfg, "ad_reduction_due.yaml") is True # type: ignore[arg-type]
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_is_price_reduction_due_returns_true_when_eligible_real(
|
||||
monkeypatch:pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""is_auto_price_reduction_due returns True when eligible (exercises real evaluate_auto_price_reduction)."""
|
||||
now_dt = datetime(2024, 6, 1, tzinfo = timezone.utc)
|
||||
monkeypatch.setattr("kleinanzeigen_bot.misc.now", lambda: now_dt)
|
||||
|
||||
price_cfg = _price_cfg(on_update = True, delay_days = 0)
|
||||
ad_cfg = SimpleNamespace(
|
||||
id = "123",
|
||||
price = 100,
|
||||
auto_price_reduction = price_cfg,
|
||||
price_reduction_count = 0,
|
||||
updated_on = datetime(2024, 1, 1, tzinfo = timezone.utc),
|
||||
repost_count = 5,
|
||||
)
|
||||
|
||||
assert kleinanzeigen_bot.is_auto_price_reduction_due(ad_cfg, "test.yaml") is True # type: ignore[arg-type]
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_is_price_reduction_due_returns_false_when_delay_not_satisfied_real(
|
||||
monkeypatch:pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""is_auto_price_reduction_due returns False when day-delay is not satisfied (exercises real evaluate_auto_price_reduction)."""
|
||||
now_dt = datetime(2024, 1, 1, 12, 0, 0, tzinfo = timezone.utc)
|
||||
monkeypatch.setattr("kleinanzeigen_bot.misc.now", lambda: now_dt)
|
||||
|
||||
price_cfg = _price_cfg(on_update = True, delay_days = 1)
|
||||
ad_cfg = SimpleNamespace(
|
||||
id = "123",
|
||||
price = 100,
|
||||
auto_price_reduction = price_cfg,
|
||||
price_reduction_count = 0,
|
||||
updated_on = datetime(2024, 1, 1, tzinfo = timezone.utc),
|
||||
repost_count = 5,
|
||||
)
|
||||
|
||||
assert kleinanzeigen_bot.is_auto_price_reduction_due(ad_cfg, "test.yaml") is False # type: ignore[arg-type]
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_is_price_reduction_due_returns_false_when_on_update_disabled_real(
|
||||
monkeypatch:pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""is_auto_price_reduction_due returns False when on_update is False (exercises real evaluate_auto_price_reduction)."""
|
||||
now_dt = datetime(2024, 6, 1, tzinfo = timezone.utc)
|
||||
monkeypatch.setattr("kleinanzeigen_bot.misc.now", lambda: now_dt)
|
||||
|
||||
price_cfg = _price_cfg(on_update = False, delay_days = 0)
|
||||
ad_cfg = SimpleNamespace(
|
||||
id = "123",
|
||||
price = 100,
|
||||
auto_price_reduction = price_cfg,
|
||||
price_reduction_count = 0,
|
||||
updated_on = datetime(2024, 1, 1, tzinfo = timezone.utc),
|
||||
repost_count = 5,
|
||||
)
|
||||
|
||||
assert kleinanzeigen_bot.is_auto_price_reduction_due(ad_cfg, "test.yaml") is False # type: ignore[arg-type]
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_evaluate_with_null_config_returns_disabled(
|
||||
) -> None:
|
||||
|
||||
Reference in New Issue
Block a user