From b7fdcdd5522f15347b0a0fe449531c1a3e8331a1 Mon Sep 17 00:00:00 2001 From: RomanAndr Date: Wed, 19 Aug 2026 21:01:13 +0300 Subject: [PATCH 1/3] fix: align analysis count with filters --- src/pharmparser/domain/analysis.py | 27 +++++++++------------------ src/pharmparser/export/grids.py | 6 +++--- tests/fixtures/golden_grids.json | 4 ++-- tests/unit/test_domain_analysis.py | 24 ++++++++++++++++++++++-- tests/unit/test_grids.py | 2 +- uv.lock | 2 +- 6 files changed, 38 insertions(+), 27 deletions(-) diff --git a/src/pharmparser/domain/analysis.py b/src/pharmparser/domain/analysis.py index 59b0acb..4c6a894 100644 --- a/src/pharmparser/domain/analysis.py +++ b/src/pharmparser/domain/analysis.py @@ -107,26 +107,17 @@ def comparison_rows(table: PriceTable, difference: DifferenceFn) -> list[Compari def count_cheapest_everywhere(table: PriceTable) -> int: - """Items the reference stocks strictly below every competitor that also stocks them. + """Items that pass ``Apply Filters`` on every displayed difference column. - Competitors that do not stock the item are ignored rather than counted as - infinitely cheap — that inversion was bug B2, which silently forced this metric - to 0 whenever any competitor lacked the item. + Excel combines those filters with AND, so a missing competitor price excludes + the row. The comparison uses the rounded values written to the workbook to keep + the dashboard count identical to the visible filtered result. """ - reference = table.reference - total = 0 - for item, price in table.prices_for(reference).items(): - competitor_prices = [ - competitor_price - for competitor in table.competitors - if (competitor_price := table.price_of(competitor, item)) is not None - ] - # ``all([])`` is true, but an item with no competing offer is unique, - # not a price victory. Keeping these metrics mutually exclusive makes - # the dashboard totals intelligible. - if competitor_prices and all(price < competitor_price for competitor_price in competitor_prices): - total += 1 - return total + return sum( + 1 + for row in comparison_rows(table, absolute_difference) + if row.differences and all(difference is not None and difference > 0 for difference in row.differences) + ) def count_unique_items(table: PriceTable) -> int: diff --git a/src/pharmparser/export/grids.py b/src/pharmparser/export/grids.py index fe7f65e..4b70b75 100644 --- a/src/pharmparser/export/grids.py +++ b/src/pharmparser/export/grids.py @@ -183,7 +183,7 @@ def build_analysis_grid(settings: ExportSettings, table: PriceTable) -> Grid: _round_metric(summary.mean_price), "Дешевле всех", summary.cheapest_everywhere, - "Только у нас", + "Уникальные товары", summary.unique_items, ), ( @@ -233,8 +233,8 @@ def build_analysis_grid(settings: ExportSettings, table: PriceTable) -> Grid: *breakdown, (), ( - "«Дешевле всех» учитывает только позиции хотя бы с одной ценой конкурента; " - "уникальные позиции считаются отдельно.", + "«Дешевле всех» совпадает с Apply Filters: цена есть у всех конкурентов и каждая разница > 0. " + "«Уникальные товары» есть только у базовой аптеки.", ), ), width=ANALYSIS_COLUMNS, diff --git a/tests/fixtures/golden_grids.json b/tests/fixtures/golden_grids.json index 72c0443..28bd3fb 100644 --- a/tests/fixtures/golden_grids.json +++ b/tests/fixtures/golden_grids.json @@ -166,7 +166,7 @@ 3.33, "Дешевле всех", 1, - "Только у нас", + "Уникальные товары", 1 ], [ @@ -243,7 +243,7 @@ ], [], [ - "«Дешевле всех» учитывает только позиции хотя бы с одной ценой конкурента; уникальные позиции считаются отдельно." + "«Дешевле всех» совпадает с Apply Filters: цена есть у всех конкурентов и каждая разница > 0. «Уникальные товары» есть только у базовой аптеки." ] ] } diff --git a/tests/unit/test_domain_analysis.py b/tests/unit/test_domain_analysis.py index 7d13c4d..51771ca 100644 --- a/tests/unit/test_domain_analysis.py +++ b/tests/unit/test_domain_analysis.py @@ -55,8 +55,8 @@ def test_differences_are_rounded_to_two_decimals() -> None: assert row.differences == (33.33,) -def test_cheapest_everywhere_ignores_missing_offers_but_not_every_offer(table: PriceTable) -> None: - """An exclusive item is not also presented as a price win. +def test_cheapest_everywhere_requires_every_competitor_offer(table: PriceTable) -> None: + """The metric follows the AND filters applied to every difference column. Аспирин (5.00 vs 6.50 and 7.00) is cheapest. Цитрамон has no comparable competitor price and belongs only in the separate unique-items metric. @@ -64,6 +64,26 @@ def test_cheapest_everywhere_ignores_missing_offers_but_not_every_offer(table: P assert count_cheapest_everywhere(table) == 1 +def test_cheapest_everywhere_matches_apply_filters_result() -> None: + table = PriceTable.build( + [ + (Pharmacy("1", "A"), {"all-positive": 5.0, "missing-offer": 5.0, "rounds-to-zero": 5.0}), + (Pharmacy("2", "B"), {"all-positive": 6.0, "missing-offer": 6.0, "rounds-to-zero": 5.004}), + (Pharmacy("3", "C"), {"all-positive": 7.0, "rounds-to-zero": 6.0}), + ] + ) + + rows = comparison_rows(table, absolute_difference) + apply_filters_result = sum( + 1 + for row in rows + if row.differences and all(difference is not None and difference > 0 for difference in row.differences) + ) + + assert apply_filters_result == 1 + assert count_cheapest_everywhere(table) == apply_filters_result + + def test_cheapest_everywhere_requires_a_strict_win() -> None: table = PriceTable.build([(Pharmacy("1", "A"), {"x": 5.0}), (Pharmacy("2", "B"), {"x": 5.0})]) assert count_cheapest_everywhere(table) == 0 diff --git a/tests/unit/test_grids.py b/tests/unit/test_grids.py index cabef58..f48aa2e 100644 --- a/tests/unit/test_grids.py +++ b/tests/unit/test_grids.py @@ -115,7 +115,7 @@ def test_analysis_grid_reports_the_headline_metrics(settings: ExportSettings, ta None, 2, ] - assert rows[4] == ["Ассортимент", 3, "Средняя цена, BYN", 3.33, "Дешевле всех", 1, "Только у нас", 1] + assert rows[4] == ["Ассортимент", 3, "Средняя цена, BYN", 3.33, "Дешевле всех", 1, "Уникальные товары", 1] assert rows[5] == [ "Позиций на рынке", 4, diff --git a/uv.lock b/uv.lock index 47d1f67..67f0f60 100644 --- a/uv.lock +++ b/uv.lock @@ -902,7 +902,7 @@ wheels = [ [[package]] name = "pharmparser" -version = "0.2.1" +version = "0.3.0" source = { editable = "." } dependencies = [ { name = "aiohttp" }, From 9ff6e659087b02ba299e1c48766bacf00e72a40a Mon Sep 17 00:00:00 2001 From: RomanAndr Date: Wed, 19 Aug 2026 21:43:28 +0300 Subject: [PATCH 2/3] fix: keep unique items visible in analysis --- src/pharmparser/export/grids.py | 4 ++-- src/pharmparser/export/xlsx_writer.py | 4 ++-- tests/fixtures/golden_grids.json | 6 +++--- tests/integration/test_workbook_roundtrip.py | 13 +++++++++---- tests/unit/test_grids.py | 2 +- uv.lock | 2 +- 6 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/pharmparser/export/grids.py b/src/pharmparser/export/grids.py index 4b70b75..393c896 100644 --- a/src/pharmparser/export/grids.py +++ b/src/pharmparser/export/grids.py @@ -179,12 +179,12 @@ def build_analysis_grid(settings: ExportSettings, table: PriceTable) -> Grid: ( "Ассортимент", summary.assortment, - "Средняя цена, BYN", - _round_metric(summary.mean_price), "Дешевле всех", summary.cheapest_everywhere, "Уникальные товары", summary.unique_items, + "Средняя цена, BYN", + _round_metric(summary.mean_price), ), ( "Позиций на рынке", diff --git a/src/pharmparser/export/xlsx_writer.py b/src/pharmparser/export/xlsx_writer.py index 0f944c5..a2febb3 100644 --- a/src/pharmparser/export/xlsx_writer.py +++ b/src/pharmparser/export/xlsx_writer.py @@ -161,9 +161,9 @@ def _style_analysis(grid: Grid, ws: Worksheet) -> None: cell.alignment = Alignment(vertical="center", wrap_text=True) ws.row_dimensions[row].height = 28 - for cell in ("B5", "F5", "H5", "B6", "F6", "H6", "B9", "D9", "F9", "H9"): + for cell in ("B5", "D5", "F5", "B6", "F6", "H6", "B9", "D9", "F9", "H9"): ws[cell].number_format = "0" - ws["D5"].number_format = '0.00 "BYN"' + ws["H5"].number_format = '0.00 "BYN"' ws["D6"].number_format = "0.00" ws["B10"].number_format = '0.00 "BYN"' for cell in ("D10", "F10"): diff --git a/tests/fixtures/golden_grids.json b/tests/fixtures/golden_grids.json index 28bd3fb..a446e17 100644 --- a/tests/fixtures/golden_grids.json +++ b/tests/fixtures/golden_grids.json @@ -162,12 +162,12 @@ [ "Ассортимент", 3, - "Средняя цена, BYN", - 3.33, "Дешевле всех", 1, "Уникальные товары", - 1 + 1, + "Средняя цена, BYN", + 3.33 ], [ "Позиций на рынке", diff --git a/tests/integration/test_workbook_roundtrip.py b/tests/integration/test_workbook_roundtrip.py index 2d4228a..95e3e40 100644 --- a/tests/integration/test_workbook_roundtrip.py +++ b/tests/integration/test_workbook_roundtrip.py @@ -37,8 +37,13 @@ def test_workbook_saves_and_reloads(tmp_path: Path, settings: ExportSettings, ta analysis = reloaded[settings.title] assert analysis["A1"].value == "АНАЛИЗ ЦЕН И АССОРТИМЕНТА" - assert analysis["F5"].value == 1 # unique items no longer also count as cheapest - assert analysis["H5"].value == 1 + assert analysis["D5"].value == 1 # unique items no longer also count as cheapest + assert analysis["E5"].value == "Уникальные товары" + assert analysis["F5"].value == 1 + assert analysis["H5"].value == 3.33 + assert analysis["D5"].number_format == "0" + assert analysis["F5"].number_format == "0" + assert analysis["H5"].number_format == '0.00 "BYN"' def test_analysis_sheet_is_a_styled_dashboard(tmp_path: Path, settings: ExportSettings, table: PriceTable) -> None: @@ -97,8 +102,8 @@ def test_single_pharmacy_workbook_is_still_valid(tmp_path: Path, settings: Expor assert [cell.value for cell in reloaded["Данные"][3]] == ["Название", "Аптека 1"] analysis = reloaded[settings.title] assert analysis["D6"].value == 0 # mean competitor assortment - assert analysis["F5"].value == 0 # no market comparison is possible - assert analysis["H5"].value == 0 + assert analysis["D5"].value == 0 # no market comparison is possible + assert analysis["F5"].value == 0 def test_xlsx_exporter_writes_to_the_configured_file_name( diff --git a/tests/unit/test_grids.py b/tests/unit/test_grids.py index f48aa2e..43b2a1d 100644 --- a/tests/unit/test_grids.py +++ b/tests/unit/test_grids.py @@ -115,7 +115,7 @@ def test_analysis_grid_reports_the_headline_metrics(settings: ExportSettings, ta None, 2, ] - assert rows[4] == ["Ассортимент", 3, "Средняя цена, BYN", 3.33, "Дешевле всех", 1, "Уникальные товары", 1] + assert rows[4] == ["Ассортимент", 3, "Дешевле всех", 1, "Уникальные товары", 1, "Средняя цена, BYN", 3.33] assert rows[5] == [ "Позиций на рынке", 4, diff --git a/uv.lock b/uv.lock index 67f0f60..b80d6f8 100644 --- a/uv.lock +++ b/uv.lock @@ -902,7 +902,7 @@ wheels = [ [[package]] name = "pharmparser" -version = "0.3.0" +version = "0.3.1" source = { editable = "." } dependencies = [ { name = "aiohttp" }, From f92f835ca6e1ff5846182e73988f2b4809a78591 Mon Sep 17 00:00:00 2001 From: RomanAndr Date: Wed, 19 Aug 2026 23:11:58 +0300 Subject: [PATCH 3/3] feat: confirm pharmacy deletion and migrate models --- docs/REFACTOR_PLAN.md | 5 +- src/pharmparser/domain/analysis.py | 18 +++-- src/pharmparser/domain/models.py | 24 ++++--- src/pharmparser/export/__init__.py | 5 +- src/pharmparser/export/grids.py | 17 +++-- src/pharmparser/export/vba/xlsm.py | 8 ++- src/pharmparser/scraping/parser.py | 7 +- src/pharmparser/ui/entry.py | 39 ++++++++++- src/pharmparser/ui/profile.py | 21 ++++-- src/pharmparser/update.py | 8 ++- tests/endpoint.py | 17 +++-- tests/fakes.py | 35 ++++++---- tests/integration/test_gui.py | 42 +++++++++++ tests/integration/test_workbook_roundtrip.py | 4 +- tests/unit/test_domain_analysis.py | 28 +++++--- tests/unit/test_domain_models.py | 15 ++-- tests/unit/test_parser.py | 40 ++++++++--- tests/unit/test_ui_entry.py | 73 ++++++++++++++++++++ tests/unit/test_update.py | 3 +- 19 files changed, 318 insertions(+), 91 deletions(-) create mode 100644 tests/unit/test_ui_entry.py diff --git a/docs/REFACTOR_PLAN.md b/docs/REFACTOR_PLAN.md index 6fd5d47..501a3d9 100644 --- a/docs/REFACTOR_PLAN.md +++ b/docs/REFACTOR_PLAN.md @@ -369,7 +369,7 @@ project that knows openpyxl; `export/vba/` holds the Windows-only macro buttons, `MacroExporter`, so the CLI and the UI pick a backend with `select_exporter()` instead of each branching on `supports_excel_macros()` themselves. -A `Grid` is a frozen dataclass of rows plus layout — column widths by index, the header +A `Grid` is a frozen Pydantic model of rows plus layout — column widths by index, the header row, the difference columns, the two conditional-format colours — which is what makes the report assertable as data. `tests/fixtures/golden_grids.json` pins all three sheets; a diff there is the diff a user would see in Excel. @@ -533,7 +533,8 @@ Everything was moved to its latest release, which surfaced three problems worth 4. **Python 3.14** is the target runtime (added 2026-08-18). This forced an `lxml` floor of 6.0.1, the first release shipping cp314 wheels. 5. **pydantic and pydantic-settings** are adopted for configuration (added 2026-08-18), which - pulled phase 4 forward ahead of phase 3. + pulled phase 4 forward ahead of phase 3. As of 2026-08-19, every structured production and + test model uses Pydantic; the project no longer uses `dataclasses`. 6. **The interface stays Russian-only.** No message catalogue, no localisation layer: the sheet labels stay as Russian literals in `export/grids.py`, where they read as the report's own wording rather than as keys pointing somewhere else. diff --git a/src/pharmparser/domain/analysis.py b/src/pharmparser/domain/analysis.py index 4c6a894..871b111 100644 --- a/src/pharmparser/domain/analysis.py +++ b/src/pharmparser/domain/analysis.py @@ -8,9 +8,10 @@ from __future__ import annotations from collections.abc import Callable -from dataclasses import dataclass from statistics import mean +from pydantic import BaseModel, ConfigDict + from .models import Pharmacy, PriceTable DifferenceFn = Callable[[float, float], float] @@ -32,8 +33,7 @@ def percentage_difference(reference: float, other: float) -> float: return (other - reference) / reference * 100 -@dataclass(frozen=True, slots=True) -class ComparisonRow: +class ComparisonRow(BaseModel): """One item across every pharmacy. ``prices`` is parallel to ``PriceTable.pharmacies``; ``differences`` is parallel @@ -42,13 +42,16 @@ class ComparisonRow: (B9: the old code wrote 0 for both). """ + model_config = ConfigDict(frozen=True, extra="forbid") + item: str prices: tuple[float | None, ...] differences: tuple[float | None, ...] -@dataclass(frozen=True, slots=True) -class CompetitorStats: +class CompetitorStats(BaseModel): + model_config = ConfigDict(frozen=True, extra="forbid") + pharmacy: Pharmacy assortment: int shared: int @@ -68,8 +71,9 @@ class CompetitorStats: """Mean competitor minus reference price for shared items, in percent.""" -@dataclass(frozen=True, slots=True) -class MarketSummary: +class MarketSummary(BaseModel): + model_config = ConfigDict(frozen=True, extra="forbid") + reference: Pharmacy assortment: int market_assortment: int diff --git a/src/pharmparser/domain/models.py b/src/pharmparser/domain/models.py index b151905..c0df8e8 100644 --- a/src/pharmparser/domain/models.py +++ b/src/pharmparser/domain/models.py @@ -1,17 +1,18 @@ """Core domain model. -Deliberately free of I/O and of every framework the app uses: no openpyxl, no -customtkinter, no COM, no network. Everything here is directly unit-testable. +Deliberately free of I/O and presentation concerns: no openpyxl, no customtkinter, +no COM, no network. Everything here is directly unit-testable. """ from __future__ import annotations from collections.abc import Iterable, Mapping, Sequence -from dataclasses import dataclass +from typing import Self +from pydantic import BaseModel, ConfigDict, model_validator -@dataclass(frozen=True, slots=True) -class Pharmacy: + +class Pharmacy(BaseModel): """A pharmacy whose prices are being compared. ``id`` is the stable identity (the numeric id from the tabletka.by URL) and is @@ -20,23 +21,27 @@ class Pharmacy: conflating the two misaligned the exported sheet. """ + model_config = ConfigDict(frozen=True, extra="forbid") + id: str name: str -@dataclass(frozen=True, slots=True) -class PriceTable: +class PriceTable(BaseModel): """Prices for a set of pharmacies. The first pharmacy is the *reference*: every comparison in :mod:`pharmparser.domain.analysis` is expressed relative to it. """ + model_config = ConfigDict(frozen=True, extra="forbid") + pharmacies: tuple[Pharmacy, ...] prices: Mapping[str, Mapping[str, float]] """Pharmacy id -> item name -> price.""" - def __post_init__(self) -> None: + @model_validator(mode="after") + def _validate_pharmacy_ids(self) -> Self: if not self.pharmacies: raise ValueError("a price table needs at least one pharmacy") ids = [pharmacy.id for pharmacy in self.pharmacies] @@ -46,6 +51,7 @@ def __post_init__(self) -> None: missing = [pid for pid in ids if pid not in self.prices] if missing: raise ValueError(f"no prices supplied for pharmacy ids: {missing}") + return self @classmethod def build(cls, entries: Iterable[tuple[Pharmacy, Mapping[str, float]]]) -> PriceTable: @@ -54,7 +60,7 @@ def build(cls, entries: Iterable[tuple[Pharmacy, Mapping[str, float]]]) -> Price for pharmacy, item_prices in entries: pharmacies.append(pharmacy) prices[pharmacy.id] = dict(item_prices) - return cls(tuple(pharmacies), prices) + return cls(pharmacies=tuple(pharmacies), prices=prices) @classmethod def from_mapping(cls, names: Sequence[str], data: Mapping[str, Mapping[str, float]]) -> PriceTable: diff --git a/src/pharmparser/export/__init__.py b/src/pharmparser/export/__init__.py index 5b5299e..138fb0a 100644 --- a/src/pharmparser/export/__init__.py +++ b/src/pharmparser/export/__init__.py @@ -82,7 +82,10 @@ def _package_macros(plain: Path, built: Path, grids, sheets) -> Path: titles = [grid.title for grid in grids] project = build_project({MODULE_NAME: module_source(sheets)}, titles) specs = { - title: [ButtonSpec(button.cell_address, button.caption, button.macro.name) for button in buttons] + title: [ + ButtonSpec(cell=button.cell_address, caption=button.caption, macro=button.macro.name) + for button in buttons + ] for title, buttons in sheets.items() } return package(plain, built, project, specs, titles) diff --git a/src/pharmparser/export/grids.py b/src/pharmparser/export/grids.py index 393c896..e711594 100644 --- a/src/pharmparser/export/grids.py +++ b/src/pharmparser/export/grids.py @@ -11,12 +11,13 @@ from __future__ import annotations from collections.abc import Mapping -from dataclasses import dataclass, field + +from pydantic import BaseModel, ConfigDict, Field from ..config import DATA_SHEET, PERCENT_SHEET, ExportSettings from ..domain import DifferenceFn, PriceTable, absolute_difference, comparison_rows, percentage_difference, summarise -Cell = str | float | None +Cell = str | int | float | None """One cell's value. ``None`` is written as a genuinely empty cell.""" HEADER_OFFSET = 2 @@ -41,10 +42,11 @@ """Columns used by the redesigned analysis dashboard.""" -@dataclass(frozen=True, slots=True) -class AnalysisPresentation: +class AnalysisPresentation(BaseModel): """Semantic layout metadata for the styled analysis dashboard.""" + model_config = ConfigDict(frozen=True, extra="forbid") + merged_ranges: tuple[tuple[int, int, int, int], ...] section_rows: tuple[int, ...] metric_rows: tuple[int, ...] @@ -54,15 +56,16 @@ class AnalysisPresentation: note_rows: tuple[int, ...] -@dataclass(frozen=True, slots=True) -class Grid: +class Grid(BaseModel): """A whole sheet: its cells plus how they should be laid out.""" + model_config = ConfigDict(frozen=True, extra="forbid") + title: str rows: tuple[tuple[Cell, ...], ...] width: int """Number of columns actually carrying content.""" - column_widths: Mapping[int, float] = field(default_factory=dict) + column_widths: Mapping[int, float] = Field(default_factory=dict) """Explicit widths by 1-based column index; every other column gets the default.""" default_column_width: float = 15 header_row: int | None = None diff --git a/src/pharmparser/export/vba/xlsm.py b/src/pharmparser/export/vba/xlsm.py index 649b786..eedc1d1 100644 --- a/src/pharmparser/export/vba/xlsm.py +++ b/src/pharmparser/export/vba/xlsm.py @@ -16,9 +16,10 @@ import shutil import zipfile from collections.abc import Mapping, Sequence -from dataclasses import dataclass from pathlib import Path +from pydantic import BaseModel, ConfigDict + logger = logging.getLogger(__name__) CONTENT_TYPES = "[Content_Types].xml" @@ -39,10 +40,11 @@ _CELL = re.compile(r"^([A-Z]+)(\d+)$") -@dataclass(frozen=True, slots=True) -class ButtonSpec: +class ButtonSpec(BaseModel): """A form-control button anchored at ``cell`` that runs ``macro`` when clicked.""" + model_config = ConfigDict(frozen=True, extra="forbid") + cell: str caption: str macro: str diff --git a/src/pharmparser/scraping/parser.py b/src/pharmparser/scraping/parser.py index 0611782..242da86 100644 --- a/src/pharmparser/scraping/parser.py +++ b/src/pharmparser/scraping/parser.py @@ -26,10 +26,10 @@ import logging import re from collections.abc import Sequence -from dataclasses import dataclass from lxml import html as lxml_html from lxml.etree import XPath, _Element +from pydantic import BaseModel, ConfigDict logger = logging.getLogger(__name__) @@ -56,8 +56,9 @@ def _has_class(name: str) -> str: _NUMBER = re.compile(r"-?\d+(?:[.,]\d+)?") -@dataclass(frozen=True, slots=True) -class DrugPrice: +class DrugPrice(BaseModel): + model_config = ConfigDict(frozen=True, extra="forbid") + name: str price: float diff --git a/src/pharmparser/ui/entry.py b/src/pharmparser/ui/entry.py index 568eb17..1f61eac 100644 --- a/src/pharmparser/ui/entry.py +++ b/src/pharmparser/ui/entry.py @@ -2,32 +2,69 @@ from __future__ import annotations +from collections.abc import Callable from typing import Any +from customtkinter import CTkButton + from .widgets import create_custom_entry +DELETE_CONFIRMATION_MS = 5_000 + class Entry: def __init__( self, parent: Any, + on_delete: Callable[[Entry], None], text_placeholder: str = "Pharmacy Name", url_placeholder: str = "https://tabletka.by/pharmacies/****", initial_text: str = "", initial_url: str = "", ) -> None: + self.parent = parent + self.on_delete = on_delete + self.delete_confirmation_id: str | None = None self.text = create_custom_entry(parent, text_placeholder, initial_text) self.url = create_custom_entry(parent, url_placeholder, initial_url) + self.delete_button = CTkButton(parent, text="✕", width=32, command=self.request_delete) def grid( self, text_row: int, url_row: int, column: int, padx: Any, pady: Any, sticky: str ) -> None: self.text.grid(row=text_row, column=column, padx=padx, pady=pady, sticky=sticky) self.url.grid(row=url_row, column=column + 1, padx=padx, pady=pady, sticky=sticky) + self.delete_button.grid(row=text_row, column=column + 2, padx=(5, 0), pady=pady) - def destroy(self) -> None: + def hide(self) -> None: self.text.grid_forget() self.url.grid_forget() + self.delete_button.grid_forget() + + def request_delete(self) -> None: + """Require a second click within five seconds before deleting the row.""" + if self.delete_confirmation_id is None: + self.delete_button.configure(text="✓") + self.delete_confirmation_id = self.parent.after( + DELETE_CONFIRMATION_MS, self.reset_delete_confirmation + ) + return + + self.parent.after_cancel(self.delete_confirmation_id) + self.delete_confirmation_id = None + self.on_delete(self) + + def reset_delete_confirmation(self) -> None: + self.delete_confirmation_id = None + self.delete_button.configure(text="✕") + + def destroy(self) -> None: + if self.delete_confirmation_id is not None: + self.parent.after_cancel(self.delete_confirmation_id) + self.delete_confirmation_id = None + self.text.destroy() + self.url.destroy() + self.delete_button.destroy() def get_text(self) -> str: return self.text.get() diff --git a/src/pharmparser/ui/profile.py b/src/pharmparser/ui/profile.py index cf47a7b..ea02459 100644 --- a/src/pharmparser/ui/profile.py +++ b/src/pharmparser/ui/profile.py @@ -14,7 +14,8 @@ def __init__(self, parent: Any, config: ProfileConfig) -> None: self.parent = parent self.name = config.name self.entries = [ - Entry(parent, initial_text=entry.name, initial_url=entry.url) for entry in config.pharmacies + Entry(parent, self.delete_entry, initial_text=entry.name, initial_url=entry.url) + for entry in config.pharmacies ] def to_config(self) -> ProfileConfig: @@ -28,17 +29,23 @@ def to_config(self) -> ProfileConfig: def hide(self) -> None: for entry in self.entries: - entry.destroy() + entry.hide() def display(self) -> None: for i, entry in enumerate(self.entries): entry.grid(text_row=i + 2, url_row=i + 2, column=0, padx=(5, 0), pady=(5, 5), sticky="nsew") def add_entry(self) -> None: - self.entries.append(Entry(self.parent)) + self.entries.append(Entry(self.parent, self.delete_entry)) self.display() - def delete_entry(self) -> None: - if self.entries: - self.entries.pop().destroy() - self.display() + def delete_entry(self, entry: Entry | None = None) -> None: + if not self.entries: + return + + target = entry or self.entries[-1] + if target not in self.entries: + return + self.entries.remove(target) + target.destroy() + self.display() diff --git a/src/pharmparser/update.py b/src/pharmparser/update.py index 58430cd..ada8ab4 100644 --- a/src/pharmparser/update.py +++ b/src/pharmparser/update.py @@ -27,10 +27,11 @@ import sys import tempfile import urllib.request -from dataclasses import dataclass from pathlib import Path from urllib.parse import urlparse +from pydantic import BaseModel, ConfigDict + from . import __version__ logger = logging.getLogger(__name__) @@ -57,10 +58,11 @@ class UpdateError(RuntimeError): """An update could not be checked for, downloaded, or installed.""" -@dataclass(frozen=True, slots=True) -class Release: +class Release(BaseModel): """A published release and the binary it offers for this platform.""" + model_config = ConfigDict(frozen=True, extra="forbid") + version: str tag: str page_url: str diff --git a/tests/endpoint.py b/tests/endpoint.py index 8395677..69ebc6e 100644 --- a/tests/endpoint.py +++ b/tests/endpoint.py @@ -15,17 +15,19 @@ import json import threading from collections.abc import Iterator, Mapping -from dataclasses import dataclass from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from urllib.parse import parse_qs +from pydantic import BaseModel, ConfigDict + DEFAULT_PHARMACY_HTML = "" -@dataclass(frozen=True) -class ReceivedRequest: +class ReceivedRequest(BaseModel): """One request the endpoint was sent.""" + model_config = ConfigDict(frozen=True, extra="forbid") + method: str path: str headers: Mapping[str, str] @@ -44,8 +46,9 @@ def page(self) -> str: return self.form.get("page", "") -@dataclass -class _Page: +class _Page(BaseModel): + model_config = ConfigDict(extra="forbid") + html: str price_count: int @@ -70,11 +73,11 @@ def __init__(self) -> None: def serve(self, pharmacy_id: str, html: str, price_count: int = 1) -> None: """Answer requests for one pharmacy with ``html``.""" - self._pages[pharmacy_id] = _Page(html, price_count) + self._pages[pharmacy_id] = _Page(html=html, price_count=price_count) def serve_all(self, html: str, price_count: int = 1) -> None: """Answer every pharmacy the same way.""" - self._default = _Page(html, price_count) + self._default = _Page(html=html, price_count=price_count) def fail(self, status: int = 500) -> None: """Answer everything with ``status`` from now on.""" diff --git a/tests/fakes.py b/tests/fakes.py index cf0ee61..1311812 100644 --- a/tests/fakes.py +++ b/tests/fakes.py @@ -8,10 +8,11 @@ from __future__ import annotations import shutil -from dataclasses import dataclass, field from pathlib import Path from typing import Any +from pydantic import BaseModel, ConfigDict, Field + class FakeCharacters: def __init__(self, frame: FakeTextFrame) -> None: @@ -36,27 +37,30 @@ def Characters(self) -> FakeCharacters: return FakeCharacters(self) -@dataclass -class FakeColour: +class FakeColour(BaseModel): + model_config = ConfigDict(extra="forbid") + RGB: int | None = None -@dataclass -class FakeFill: - BackColor: FakeColour = field(default_factory=FakeColour) - ForeColor: FakeColour = field(default_factory=FakeColour) +class FakeFill(BaseModel): + model_config = ConfigDict(extra="forbid") + + BackColor: FakeColour = Field(default_factory=FakeColour) + ForeColor: FakeColour = Field(default_factory=FakeColour) -@dataclass -class FakeShape: +class FakeShape(BaseModel): + model_config = ConfigDict(arbitrary_types_allowed=True, extra="forbid") + Name: str Left: float Top: float Width: float Height: float OnAction: str | None = None - TextFrame: FakeTextFrame = field(default_factory=FakeTextFrame) - Fill: FakeFill = field(default_factory=FakeFill) + TextFrame: FakeTextFrame = Field(default_factory=FakeTextFrame) + Fill: FakeFill = Field(default_factory=FakeFill) class FakeShapes: @@ -64,13 +68,16 @@ def __init__(self) -> None: self.shapes: list[FakeShape] = [] def AddShape(self, _kind: int, left: float, top: float, width: float, height: float) -> FakeShape: - shape = FakeShape(f"Shape {len(self.shapes) + 1}", left, top, width, height) + shape = FakeShape( + Name=f"Shape {len(self.shapes) + 1}", Left=left, Top=top, Width=width, Height=height + ) self.shapes.append(shape) return shape -@dataclass -class FakeCell: +class FakeCell(BaseModel): + model_config = ConfigDict(extra="forbid") + Left: float Top: float Width: float diff --git a/tests/integration/test_gui.py b/tests/integration/test_gui.py index d74557d..9bb01d3 100644 --- a/tests/integration/test_gui.py +++ b/tests/integration/test_gui.py @@ -21,11 +21,13 @@ import sys import threading import time +from collections.abc import Callable from pathlib import Path import pytest from pharmparser.scraping import ScrapeError +from pharmparser.ui.entry import DELETE_CONFIRMATION_MS NEEDS_X11 = sys.platform not in ("win32", "darwin") HAS_DISPLAY = not NEEDS_X11 or bool(os.environ.get("DISPLAY") or os.environ.get("WAYLAND_DISPLAY")) @@ -87,6 +89,46 @@ def test_add_and_delete_entry(app) -> None: assert len(app.current_profile.entries) == 1 +def test_entry_delete_requires_two_clicks(app, monkeypatch: pytest.MonkeyPatch) -> None: + app.add_entry() + first, second = app.current_profile.entries + cancelled: list[str] = [] + monkeypatch.setattr(app, "after", lambda delay, callback: "delete-confirmation") + monkeypatch.setattr(app, "after_cancel", cancelled.append) + + first.delete_button.invoke() + + assert first.delete_button.cget("text") == "✓" + assert app.current_profile.entries == [first, second] + + first.delete_button.invoke() + + assert app.current_profile.entries == [second] + assert cancelled == ["delete-confirmation"] + + +def test_entry_delete_confirmation_expires_after_five_seconds( + app, monkeypatch: pytest.MonkeyPatch +) -> None: + entry = app.current_profile.entries[0] + scheduled: list[tuple[int, Callable[[], None]]] = [] + + def schedule(delay: int, callback: Callable[[], None]) -> str: + scheduled.append((delay, callback)) + return "delete-confirmation" + + monkeypatch.setattr(app, "after", schedule) + + entry.request_delete() + + delay, reset = scheduled[0] + assert delay == DELETE_CONFIRMATION_MS == 5_000 + reset() + assert entry.delete_button.cget("text") == "✕" + assert entry.delete_confirmation_id is None + assert app.current_profile.entries == [entry] + + def test_add_and_delete_profile(app) -> None: app.selector.add() app.update_idletasks() diff --git a/tests/integration/test_workbook_roundtrip.py b/tests/integration/test_workbook_roundtrip.py index 95e3e40..aae3ce2 100644 --- a/tests/integration/test_workbook_roundtrip.py +++ b/tests/integration/test_workbook_roundtrip.py @@ -96,7 +96,9 @@ def test_single_pharmacy_workbook_is_still_valid(tmp_path: Path, settings: Expor """A profile with one pharmacy has no difference columns at all.""" from pharmparser.domain import Pharmacy - table = PriceTable.build([(Pharmacy("1", "Аптека 1"), {"Аспирин": 5.0})]) + table = PriceTable.build( + [(Pharmacy(id="1", name="Аптека 1"), {"Аспирин": 5.0})] + ) reloaded = load_workbook(write_workbook(settings, table, tmp_path / "single.xlsx")) assert [cell.value for cell in reloaded["Данные"][3]] == ["Название", "Аптека 1"] diff --git a/tests/unit/test_domain_analysis.py b/tests/unit/test_domain_analysis.py index 51771ca..56811a3 100644 --- a/tests/unit/test_domain_analysis.py +++ b/tests/unit/test_domain_analysis.py @@ -50,7 +50,9 @@ def test_missing_prices_give_an_undefined_difference(table: PriceTable) -> None: def test_differences_are_rounded_to_two_decimals() -> None: - table = PriceTable.build([(Pharmacy("1", "A"), {"x": 3.00}), (Pharmacy("2", "B"), {"x": 4.00})]) + table = PriceTable.build( + [(Pharmacy(id="1", name="A"), {"x": 3.00}), (Pharmacy(id="2", name="B"), {"x": 4.00})] + ) (row,) = comparison_rows(table, percentage_difference) assert row.differences == (33.33,) @@ -67,9 +69,15 @@ def test_cheapest_everywhere_requires_every_competitor_offer(table: PriceTable) def test_cheapest_everywhere_matches_apply_filters_result() -> None: table = PriceTable.build( [ - (Pharmacy("1", "A"), {"all-positive": 5.0, "missing-offer": 5.0, "rounds-to-zero": 5.0}), - (Pharmacy("2", "B"), {"all-positive": 6.0, "missing-offer": 6.0, "rounds-to-zero": 5.004}), - (Pharmacy("3", "C"), {"all-positive": 7.0, "rounds-to-zero": 6.0}), + ( + Pharmacy(id="1", name="A"), + {"all-positive": 5.0, "missing-offer": 5.0, "rounds-to-zero": 5.0}, + ), + ( + Pharmacy(id="2", name="B"), + {"all-positive": 6.0, "missing-offer": 6.0, "rounds-to-zero": 5.004}, + ), + (Pharmacy(id="3", name="C"), {"all-positive": 7.0, "rounds-to-zero": 6.0}), ] ) @@ -85,7 +93,9 @@ def test_cheapest_everywhere_matches_apply_filters_result() -> None: def test_cheapest_everywhere_requires_a_strict_win() -> None: - table = PriceTable.build([(Pharmacy("1", "A"), {"x": 5.0}), (Pharmacy("2", "B"), {"x": 5.0})]) + table = PriceTable.build( + [(Pharmacy(id="1", name="A"), {"x": 5.0}), (Pharmacy(id="2", name="B"), {"x": 5.0})] + ) assert count_cheapest_everywhere(table) == 0 @@ -94,7 +104,7 @@ def test_unique_items_counts_only_the_reference(table: PriceTable) -> None: def test_a_single_pharmacy_cannot_claim_market_wins_or_unique_items() -> None: - table = PriceTable.build([(Pharmacy("1", "A"), {"x": 1.0})]) + table = PriceTable.build([(Pharmacy(id="1", name="A"), {"x": 1.0})]) assert count_cheapest_everywhere(table) == 0 assert count_unique_items(table) == 0 @@ -155,8 +165,8 @@ def test_summary_competitor_breakdown(table: PriceTable) -> None: def test_equal_prices_are_a_separate_comparison_outcome() -> None: table = PriceTable.build( [ - (Pharmacy("1", "A"), {"same": 5.0, "only-a": 2.0}), - (Pharmacy("2", "B"), {"same": 5.0, "only-b": 7.0}), + (Pharmacy(id="1", name="A"), {"same": 5.0, "only-a": 2.0}), + (Pharmacy(id="2", name="B"), {"same": 5.0, "only-b": 7.0}), ] ) @@ -170,7 +180,7 @@ def test_equal_prices_are_a_separate_comparison_outcome() -> None: def test_summary_survives_a_single_pharmacy() -> None: """statistics.mean raises on an empty sequence where numpy returned nan.""" - table = PriceTable.build([(Pharmacy("1", "A"), {"x": 1.0})]) + table = PriceTable.build([(Pharmacy(id="1", name="A"), {"x": 1.0})]) summary = summarise(table) assert summary.mean_competitor_assortment == 0 assert summary.competitors == () diff --git a/tests/unit/test_domain_models.py b/tests/unit/test_domain_models.py index 35b2601..c8fd94e 100644 --- a/tests/unit/test_domain_models.py +++ b/tests/unit/test_domain_models.py @@ -26,7 +26,9 @@ def test_item_names_are_the_union_sorted_case_insensitively(table: PriceTable) - def test_item_names_sorting_ignores_case() -> None: - table = PriceTable.build([(Pharmacy("1", "A"), {"banana": 1.0, "Apple": 2.0, "cherry": 3.0})]) + table = PriceTable.build( + [(Pharmacy(id="1", name="A"), {"banana": 1.0, "Apple": 2.0, "cherry": 3.0})] + ) assert table.item_names() == ["Apple", "banana", "cherry"] @@ -42,13 +44,18 @@ def test_empty_table_is_rejected() -> None: def test_duplicate_pharmacy_ids_are_rejected() -> None: with pytest.raises(ValueError, match="duplicate pharmacy ids"): - PriceTable.build([(Pharmacy("1", "A"), {}), (Pharmacy("1", "B"), {})]) + PriceTable.build( + [(Pharmacy(id="1", name="A"), {}), (Pharmacy(id="1", name="B"), {})] + ) def test_pharmacies_may_share_a_display_name_when_ids_differ() -> None: """B14: identity is the id, so two branches of one chain are representable.""" table = PriceTable.build( - [(Pharmacy("1", "Аптека"), {"Аспирин": 5.0}), (Pharmacy("2", "Аптека"), {"Аспирин": 6.0})] + [ + (Pharmacy(id="1", name="Аптека"), {"Аспирин": 5.0}), + (Pharmacy(id="2", name="Аптека"), {"Аспирин": 6.0}), + ] ) assert table.assortment(table.pharmacies[0]) == 1 assert table.price_of(table.pharmacies[1], "Аспирин") == 6.0 @@ -67,4 +74,4 @@ def test_legacy_adapter_tolerates_a_pharmacy_with_no_prices() -> None: def test_pharmacy_without_a_prices_entry_is_rejected() -> None: with pytest.raises(ValueError, match="no prices supplied"): - PriceTable(pharmacies=(Pharmacy("1", "A"),), prices={}) + PriceTable(pharmacies=(Pharmacy(id="1", name="A"),), prices={}) diff --git a/tests/unit/test_parser.py b/tests/unit/test_parser.py index e478e05..f5db34c 100644 --- a/tests/unit/test_parser.py +++ b/tests/unit/test_parser.py @@ -79,10 +79,16 @@ def test_every_row_of_a_real_page_is_read(name: str) -> None: def test_a_real_page_parses_to_the_expected_values() -> None: prices = parse_page(html_of("live_price_page.json")) assert prices[0] == DrugPrice( - "9 Месяцев Фолиевая кислота, таблетки покрытые оболочкой 400мкг N30, Валента", 10.17 + name="9 Месяцев Фолиевая кислота, таблетки покрытые оболочкой 400мкг N30, Валента", + price=10.17, + ) + assert prices[1] == DrugPrice( + name="911 Дегтярное жидкое мыло, жидкое мыло 250мл N1, Твинс Тэк ЗАО", + price=6.84, + ) + assert prices[2] == DrugPrice( + name="911 Теймурова паста, паста 50мл N1, Твинс Тэк ЗАО", price=4.36 ) - assert prices[1] == DrugPrice("911 Дегтярное жидкое мыло, жидкое мыло 250мл N1, Твинс Тэк ЗАО", 6.84) - assert prices[2] == DrugPrice("911 Теймурова паста, паста 50мл N1, Твинс Тэк ЗАО", 4.36) @pytest.mark.parametrize("name", LIVE_PAGES) @@ -129,8 +135,8 @@ def test_the_test_markup_matches_the_captured_markup() -> None: def test_parses_every_row_with_name_form_and_maker() -> None: assert parse_page(simple_page("5,00 р.")) == [ - DrugPrice("Аспирин, таблетки 100мг, Производитель", 5.00), - DrugPrice("Цитрамон, таблетки N10, Производитель", 5.00), + DrugPrice(name="Аспирин, таблетки 100мг, Производитель", price=5.00), + DrugPrice(name="Цитрамон, таблетки N10, Производитель", price=5.00), ] @@ -143,8 +149,11 @@ def test_rows_stay_aligned_when_the_page_drifts() -> None: document-wide and zipped them, so every later name took the wrong price. """ assert parse_page(fixture("price_page_drifted.html")) == [ - DrugPrice("9 Месяцев Фолиевая кислота, таблетки покрытые оболочкой 400мкг N30, Валента", 10.17), - DrugPrice("911 Теймурова паста, Твинс Тэк ЗАО", 4.36), + DrugPrice( + name="9 Месяцев Фолиевая кислота, таблетки покрытые оболочкой 400мкг N30, Валента", + price=10.17, + ), + DrugPrice(name="911 Теймурова паста, Твинс Тэк ЗАО", price=4.36), ] @@ -172,20 +181,27 @@ def test_a_page_of_prices_that_reads_as_empty_is_an_error(caplog: pytest.LogCapt def test_a_row_without_a_form_title_still_yields_a_price() -> None: - assert parse_page(page(row("Аспирин", "", "5,00 р.", maker=""))) == [DrugPrice("Аспирин", 5.00)] + assert parse_page(page(row("Аспирин", "", "5,00 р.", maker=""))) == [ + DrugPrice(name="Аспирин", price=5.00) + ] # -- merging ------------------------------------------------------------------- def test_merge_flattens_pages() -> None: - pages = [[DrugPrice("A", 1.0), DrugPrice("B", 2.0)], [DrugPrice("C", 3.0)]] + pages = [ + [DrugPrice(name="A", price=1.0), DrugPrice(name="B", price=2.0)], + [DrugPrice(name="C", price=3.0)], + ] assert merge(pages) == {"A": 1.0, "B": 2.0, "C": 3.0} def test_merge_keeps_the_last_price_for_a_repeated_label() -> None: """Only a genuinely identical item — same name, pack and maker — merges now.""" - assert merge([[DrugPrice("A", 1.0)], [DrugPrice("A", 9.0)]]) == {"A": 9.0} + assert merge([[DrugPrice(name="A", price=1.0)], [DrugPrice(name="A", price=9.0)]]) == { + "A": 9.0 + } def test_two_makers_of_one_drug_stay_apart() -> None: @@ -196,7 +212,9 @@ def test_two_makers_of_one_drug_stay_apart() -> None: second = item_label("Амлодипин", "таблетки 10мг N30", "Тева") assert first != second assert first.startswith("Амлодипин, таблетки 10мг N30"), "the name still leads" - assert merge([[DrugPrice(first, 1.0), DrugPrice(second, 9.0)]]) == {first: 1.0, second: 9.0} + assert merge( + [[DrugPrice(name=first, price=1.0), DrugPrice(name=second, price=9.0)]] + ) == {first: 1.0, second: 9.0} def test_a_label_leaves_out_the_parts_it_does_not_have() -> None: diff --git a/tests/unit/test_ui_entry.py b/tests/unit/test_ui_entry.py new file mode 100644 index 0000000..18aca3b --- /dev/null +++ b/tests/unit/test_ui_entry.py @@ -0,0 +1,73 @@ +from __future__ import annotations + +from collections.abc import Callable +from typing import Any + +from pharmparser.ui.entry import DELETE_CONFIRMATION_MS, Entry + + +class FakeParent: + def __init__(self) -> None: + self.delay: int | None = None + self.callback: Callable[[], None] | None = None + self.cancelled: list[str] = [] + + def after(self, delay: int, callback: Callable[[], None]) -> str: + self.delay = delay + self.callback = callback + return "delete-confirmation" + + def after_cancel(self, confirmation_id: str) -> None: + self.cancelled.append(confirmation_id) + + +class FakeButton: + def __init__(self) -> None: + self.text = "✕" + + def configure(self, *, text: str) -> None: + self.text = text + + +def make_entry() -> tuple[Any, FakeParent, FakeButton, list[Any]]: + parent = FakeParent() + button = FakeButton() + deleted: list[Any] = [] + entry: Any = Entry.__new__(Entry) + entry.parent = parent + entry.on_delete = deleted.append + entry.delete_confirmation_id = None + entry.delete_button = button + return entry, parent, button, deleted + + +def test_first_delete_click_arms_confirmation_for_five_seconds() -> None: + entry, parent, button, deleted = make_entry() + + entry.request_delete() + + assert button.text == "✓" + assert parent.delay == DELETE_CONFIRMATION_MS == 5_000 + assert deleted == [] + + +def test_delete_confirmation_expires_without_removing_the_entry() -> None: + entry, parent, button, deleted = make_entry() + entry.request_delete() + assert parent.callback is not None + + parent.callback() + + assert button.text == "✕" + assert entry.delete_confirmation_id is None + assert deleted == [] + + +def test_second_delete_click_removes_the_entry() -> None: + entry, parent, _button, deleted = make_entry() + entry.request_delete() + + entry.request_delete() + + assert deleted == [entry] + assert parent.cancelled == ["delete-confirmation"] diff --git a/tests/unit/test_update.py b/tests/unit/test_update.py index 8bee377..695a990 100644 --- a/tests/unit/test_update.py +++ b/tests/unit/test_update.py @@ -11,7 +11,6 @@ import json import threading from collections.abc import Iterator -from dataclasses import replace from http.server import BaseHTTPRequestHandler, HTTPServer from pathlib import Path from typing import ClassVar @@ -174,7 +173,7 @@ def test_a_release_without_checksums_is_refused(api: str, monkeypatch: pytest.Mo release = update.latest_release() assert release is not None with pytest.raises(UpdateError, match="refusing to install it unverified"): - update.download(replace(release, checksums_url=None), tmp_path) + update.download(release.model_copy(update={"checksums_url": None}), tmp_path) def test_a_checksum_mismatch_is_refused(api: str, monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: