mirror of
https://github.com/Second-Hand-Friends/kleinanzeigen-bot.git
synced 2026-07-09 12:41:05 +02:00
feat: show filenames in status output (#1196)
## ℹ️ Description Show the full ad filename in `pdm run app status` output so status rows can be traced back to their source ad files. - Link to the related issue(s): none - Motivation: the status table currently shows ID, title, and status, but not the source filename, which makes it harder to identify which ad file a row came from. ## 📋 Changes Summary - Add a filename field to status rows, populated from the relative ad file path. - Render a `Filename` column in the status table. - Add the German translation for the new status table header. - Update status rendering unit tests for the new column. - No new dependencies or configuration changes. ### ⚙️ Type of Change Select the type(s) of change(s) included in this pull request: - [ ] 🐞 Bug fix (non-breaking change which fixes an issue) - [x] ✨ New feature (adds new functionality without breaking existing usage) - [ ] 💥 Breaking change (changes that might break existing user setups, scripts, or configurations) ## ✅ Checklist Before requesting a review, confirm the following: - [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 ## Summary by CodeRabbit * **New Features** * Added a **Filename** column to the status table, displaying the relative ad file path for each ad. * **Tests** * Updated automated tests to reflect the new table layout, verify correct filename values, and adjust ANSI formatting checks to account for the added column. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -27,6 +27,7 @@ class StatusRow:
|
||||
|
||||
title:str # ad.title
|
||||
ad_id:str # "-" if None, else str(ad.id)
|
||||
filename:str # Relative ad-file path (e.g. "ads/sofa.yaml")
|
||||
status:str # One of: "disabled", "draft", "changed", "due", "published-local"
|
||||
apr_repost:str | None = None # APR repost cell; ``None`` → rendered as "off"
|
||||
apr_update:str | None = None # APR update cell; ``None`` → rendered as "off"
|
||||
@@ -145,6 +146,7 @@ def build_status_rows(
|
||||
StatusRow(
|
||||
title = ad_cfg.title,
|
||||
ad_id = "-" if ad_cfg.id is None else str(ad_cfg.id),
|
||||
filename = ad_file_rel,
|
||||
status = status,
|
||||
apr_repost = apr_repost,
|
||||
apr_update = apr_update,
|
||||
@@ -193,48 +195,57 @@ def render_status_rows(rows:list[StatusRow], *, color:bool = False) -> str:
|
||||
if not rows:
|
||||
return ""
|
||||
|
||||
h_id = _("Ad ID")
|
||||
h_title = _("Title")
|
||||
h_status = _("Status")
|
||||
|
||||
col_id = max(len(h_id), max((len(r.ad_id) for r in rows), default = 0))
|
||||
|
||||
# Translated labels for column width calculation.
|
||||
col_status = max(len(h_status), *[len(_translate_status(s)) for s in _STATUS_ORDER], 0)
|
||||
|
||||
# Title width is data-driven, but clamp to at least header width
|
||||
col_title = max(len(h_title), max((len(r.title) for r in rows), default = 0))
|
||||
# Column header labels and data-driven widths.
|
||||
H = {
|
||||
"id": _("Ad ID"),
|
||||
"fn": _("Filename"),
|
||||
"title": _("Title"),
|
||||
"status": _("Status"),
|
||||
}
|
||||
W = {
|
||||
"id": max(len(H["id"]), max((len(r.ad_id) for r in rows), default = 0)),
|
||||
"fn": max(len(H["fn"]), max((len(r.filename) for r in rows), default = 0)),
|
||||
"title": max(len(H["title"]), max((len(r.title) for r in rows), default = 0)),
|
||||
"status": max(len(H["status"]), *[len(_translate_status(s)) for s in _STATUS_ORDER], 0),
|
||||
}
|
||||
|
||||
# APR columns — only if any row has non-None APR data
|
||||
apr_show, h_apr_r, h_apr_u, w_apr_r, w_apr_u = _apr_layout(rows)
|
||||
|
||||
# Build separator and header
|
||||
separator_parts = ["+", "-" * (col_id + 2), "+", "-" * (col_title + 2), "+", "-" * (col_status + 2)]
|
||||
header_parts = ["| ", h_id.ljust(col_id), " | ", h_title.ljust(col_title), " | ", h_status.ljust(col_status)]
|
||||
# Build separator and header row
|
||||
widths = [W["id"], W["fn"], W["title"], W["status"]]
|
||||
if apr_show:
|
||||
separator_parts += ["+", "-" * (w_apr_r + 2), "+", "-" * (w_apr_u + 2)]
|
||||
header_parts += [" | ", h_apr_r.ljust(w_apr_r), " | ", h_apr_u.ljust(w_apr_u)]
|
||||
separator = "".join(separator_parts) + "+"
|
||||
header = "".join(header_parts) + " |"
|
||||
widths += [w_apr_r, w_apr_u]
|
||||
sep = "".join("+" + "-" * (w + 2) for w in widths) + "+"
|
||||
|
||||
lines:list[str] = [separator, header, separator]
|
||||
hdr_parts = [
|
||||
"| ", H["id"].ljust(W["id"]), " | ", H["fn"].ljust(W["fn"]),
|
||||
" | ", H["title"].ljust(W["title"]), " | ", H["status"].ljust(W["status"]),
|
||||
]
|
||||
if apr_show:
|
||||
hdr_parts += [" | ", h_apr_r.ljust(w_apr_r), " | ", h_apr_u.ljust(w_apr_u)]
|
||||
hdr_parts.append(" |")
|
||||
hdr = "".join(hdr_parts)
|
||||
|
||||
lines:list[str] = [sep, hdr, sep]
|
||||
|
||||
for r in rows:
|
||||
label = _translate_status(r.status).ljust(col_status)
|
||||
label = _translate_status(r.status).ljust(W["status"])
|
||||
cell = _colorize_status(r.status, label) if color else label
|
||||
|
||||
row_parts = [
|
||||
"| ", r.ad_id.ljust(col_id), " | ", r.title.ljust(col_title), " | ", cell,
|
||||
parts = [
|
||||
"| ", r.ad_id.ljust(W["id"]), " | ", r.filename.ljust(W["fn"]),
|
||||
" | ", r.title.ljust(W["title"]), " | ", cell,
|
||||
]
|
||||
if apr_show:
|
||||
row_parts.extend([
|
||||
parts += [
|
||||
" | ", _apr_cell(r.apr_repost).ljust(w_apr_r),
|
||||
" | ", _apr_cell(r.apr_update).ljust(w_apr_u),
|
||||
])
|
||||
row_parts.append(" |")
|
||||
lines.append("".join(row_parts))
|
||||
]
|
||||
parts.append(" |")
|
||||
lines.append("".join(parts))
|
||||
|
||||
lines.append(separator)
|
||||
lines.append(sep)
|
||||
|
||||
# Summary line — always plain
|
||||
counts:dict[str, int] = {}
|
||||
|
||||
@@ -478,6 +478,7 @@ kleinanzeigen_bot/ad_status.py:
|
||||
|
||||
render_status_rows:
|
||||
"Ad ID": "Anzeigen-ID"
|
||||
"Filename": "Dateiname"
|
||||
"Title": "Titel"
|
||||
"Status": "Status"
|
||||
"Summary: %s (%d total)": "Zusammenfassung: %s (%d insgesamt)"
|
||||
|
||||
@@ -206,6 +206,8 @@ def test_build_status_rows() -> None:
|
||||
]
|
||||
rows = build_status_rows(ads, now = _now())
|
||||
assert len(rows) == 2
|
||||
assert rows[0].filename == "ads/one.yaml"
|
||||
assert rows[1].filename == "ads/two.yaml"
|
||||
assert rows[0].status == "disabled"
|
||||
assert rows[1].status == "draft"
|
||||
|
||||
@@ -221,8 +223,8 @@ class TestRenderStatusRows:
|
||||
|
||||
def test_headers_and_rows(self) -> None:
|
||||
rows = [
|
||||
StatusRow(title = "Ad A", ad_id = "-", status = "draft"),
|
||||
StatusRow(title = "Ad B", ad_id = "123", status = "published-local"),
|
||||
StatusRow(title = "Ad A", ad_id = "-", filename = "a.yaml", status = "draft"),
|
||||
StatusRow(title = "Ad B", ad_id = "123", filename = "b.yaml", status = "published-local"),
|
||||
]
|
||||
output = render_status_rows(rows)
|
||||
|
||||
@@ -233,12 +235,15 @@ class TestRenderStatusRows:
|
||||
|
||||
# Contains headers
|
||||
assert "Ad ID" in output
|
||||
assert "Filename" in output
|
||||
assert "Title" in output
|
||||
assert "Status" in output
|
||||
|
||||
# Contains row data
|
||||
assert "-" in output
|
||||
assert "123" in output
|
||||
assert "a.yaml" in output
|
||||
assert "b.yaml" in output
|
||||
assert "Ad A" in output
|
||||
assert "Ad B" in output
|
||||
|
||||
@@ -249,6 +254,13 @@ class TestRenderStatusRows:
|
||||
# Summary ends with total count
|
||||
assert "total" in output.casefold() or "gesamt" in output.casefold()
|
||||
|
||||
# Column order: Ad ID, Filename, Title, Status
|
||||
header_line = output.splitlines()[1]
|
||||
headers = [c.strip() for c in header_line.split("|") if c.strip()]
|
||||
assert headers == ["Ad ID", "Filename", "Title", "Status"], (
|
||||
f"Unexpected column order: {headers}"
|
||||
)
|
||||
|
||||
# ------------------------------------------------------------------ #
|
||||
# Colour rendering
|
||||
# ------------------------------------------------------------------ #
|
||||
@@ -256,25 +268,25 @@ class TestRenderStatusRows:
|
||||
def test_render_uncoloured_unchanged(self) -> None:
|
||||
"""Uncoloured output matches the default (color=False)."""
|
||||
rows = [
|
||||
StatusRow(title = "Item", ad_id = "1", status = "draft"),
|
||||
StatusRow(title = "Thing", ad_id = "2", status = "published-local"),
|
||||
StatusRow(title = "Item", ad_id = "1", filename = "a.yaml", status = "draft"),
|
||||
StatusRow(title = "Thing", ad_id = "2", filename = "b.yaml", status = "published-local"),
|
||||
]
|
||||
assert render_status_rows(rows) == render_status_rows(rows, color = False)
|
||||
|
||||
def test_render_coloured_contains_ansi(self) -> None:
|
||||
"""Coloured output includes ANSI escape sequences."""
|
||||
rows = [StatusRow(title = "Test", ad_id = "42", status = "changed")]
|
||||
rows = [StatusRow(title = "Test", ad_id = "42", filename = "t.yaml", status = "changed")]
|
||||
output = render_status_rows(rows, color = True)
|
||||
assert "\x1b[" in output
|
||||
|
||||
def test_render_coloured_stripped_equals_uncoloured(self) -> None:
|
||||
"""Stripping ANSI from coloured output produces identical text to uncoloured."""
|
||||
rows = [
|
||||
StatusRow(title = "Sofa", ad_id = "-", status = "draft"),
|
||||
StatusRow(title = "Chair", ad_id = "99", status = "published-local"),
|
||||
StatusRow(title = "Table", ad_id = "55", status = "changed"),
|
||||
StatusRow(title = "Lamp", ad_id = "33", status = "due"),
|
||||
StatusRow(title = "Rug", ad_id = "11", status = "disabled"),
|
||||
StatusRow(title = "Sofa", ad_id = "-", filename = "sofa.yaml", status = "draft"),
|
||||
StatusRow(title = "Chair", ad_id = "99", filename = "chair.yaml", status = "published-local"),
|
||||
StatusRow(title = "Table", ad_id = "55", filename = "table.yaml", status = "changed"),
|
||||
StatusRow(title = "Lamp", ad_id = "33", filename = "lamp.yaml", status = "due"),
|
||||
StatusRow(title = "Rug", ad_id = "11", filename = "rug.yaml", status = "disabled"),
|
||||
]
|
||||
|
||||
plain = render_status_rows(rows, color = False)
|
||||
@@ -287,7 +299,7 @@ class TestRenderStatusRows:
|
||||
|
||||
def test_render_coloured_only_status_column(self) -> None:
|
||||
"""Only the status column contains ANSI codes; headers and separators are plain."""
|
||||
rows = [StatusRow(title = "Desk", ad_id = "7", status = "due")]
|
||||
rows = [StatusRow(title = "Desk", ad_id = "7", filename = "desk.yaml", status = "due")]
|
||||
output = render_status_rows(rows, color = True)
|
||||
|
||||
# Header row should not contain ANSI
|
||||
@@ -308,7 +320,7 @@ class TestRenderStatusRows:
|
||||
|
||||
def test_render_colour_only_mapped_statuses(self) -> None:
|
||||
"""Only known status values get colour; unmapped statuses are unchanged."""
|
||||
rows = [StatusRow(title = "?iss", ad_id = "0", status = "unknown")]
|
||||
rows = [StatusRow(title = "?iss", ad_id = "0", filename = "q.yaml", status = "unknown")]
|
||||
plain = render_status_rows(rows, color = False)
|
||||
coloured = render_status_rows(rows, color = True)
|
||||
assert plain == coloured, "Unmapped status should not be coloured"
|
||||
@@ -327,8 +339,8 @@ class TestAprRendering:
|
||||
def test_no_apr_no_columns(self) -> None:
|
||||
"""APR columns absent when no effective APR is configured."""
|
||||
rows = [
|
||||
StatusRow(title = "A", ad_id = "1", status = "published-local"),
|
||||
StatusRow(title = "B", ad_id = "2", status = "draft"),
|
||||
StatusRow(title = "A", ad_id = "1", filename = "a.yaml", status = "published-local"),
|
||||
StatusRow(title = "B", ad_id = "2", filename = "b.yaml", status = "draft"),
|
||||
]
|
||||
output = render_status_rows(rows)
|
||||
assert "APR" not in output
|
||||
@@ -339,7 +351,7 @@ class TestAprRendering:
|
||||
def test_columns_present_when_replace_apr_enabled(self) -> None:
|
||||
"""APR columns visible when REPLACE decision has effective APR."""
|
||||
rows = [
|
||||
StatusRow(title = "A", ad_id = "1", status = "published-local", apr_repost = "due: 9"),
|
||||
StatusRow(title = "A", ad_id = "1", filename = "a.yaml", status = "published-local", apr_repost = "due: 9"),
|
||||
]
|
||||
output = render_status_rows(rows)
|
||||
assert "APR repost" in output
|
||||
@@ -349,7 +361,7 @@ class TestAprRendering:
|
||||
def test_columns_present_when_update_apr_enabled(self) -> None:
|
||||
"""APR columns visible when MODIFY decision has effective APR."""
|
||||
rows = [
|
||||
StatusRow(title = "A", ad_id = "1", status = "published-local", apr_update = "due: 7"),
|
||||
StatusRow(title = "A", ad_id = "1", filename = "a.yaml", status = "published-local", apr_update = "due: 7"),
|
||||
]
|
||||
output = render_status_rows(rows)
|
||||
assert "APR repost" in output
|
||||
@@ -361,8 +373,8 @@ class TestAprRendering:
|
||||
def test_apr_cell_off_for_none(self) -> None:
|
||||
"""None APR displayed as 'off'."""
|
||||
rows = [
|
||||
StatusRow(title = "A", ad_id = "1", status = "published-local", apr_repost = "due: 5"),
|
||||
StatusRow(title = "B", ad_id = "2", status = "published-local", apr_repost = None),
|
||||
StatusRow(title = "A", ad_id = "1", filename = "a.yaml", status = "published-local", apr_repost = "due: 5"),
|
||||
StatusRow(title = "B", ad_id = "2", filename = "b.yaml", status = "published-local", apr_repost = None),
|
||||
]
|
||||
output = render_status_rows(rows)
|
||||
assert "off" in output
|
||||
@@ -370,7 +382,7 @@ class TestAprRendering:
|
||||
def test_apr_cell_error(self) -> None:
|
||||
"""Error reason displayed as 'error'."""
|
||||
rows = [
|
||||
StatusRow(title = "A", ad_id = "1", status = "published-local", apr_repost = "error"),
|
||||
StatusRow(title = "A", ad_id = "1", filename = "a.yaml", status = "published-local", apr_repost = "error"),
|
||||
]
|
||||
output = render_status_rows(rows)
|
||||
assert "error" in output
|
||||
@@ -378,7 +390,7 @@ class TestAprRendering:
|
||||
def test_apr_cell_not_due(self) -> None:
|
||||
"""Not-due displayed."""
|
||||
rows = [
|
||||
StatusRow(title = "A", ad_id = "1", status = "published-local", apr_repost = "not due"),
|
||||
StatusRow(title = "A", ad_id = "1", filename = "a.yaml", status = "published-local", apr_repost = "not due"),
|
||||
]
|
||||
output = render_status_rows(rows)
|
||||
assert "not due" in output
|
||||
@@ -388,7 +400,7 @@ class TestAprRendering:
|
||||
def test_apr_columns_not_coloured(self) -> None:
|
||||
"""APR columns contain no ANSI codes when status is coloured."""
|
||||
rows = [
|
||||
StatusRow(title = "A", ad_id = "1", status = "published-local", apr_repost = "due: 5", apr_update = "not due"),
|
||||
StatusRow(title = "A", ad_id = "1", filename = "a.yaml", status = "published-local", apr_repost = "due: 5", apr_update = "not due"),
|
||||
]
|
||||
output = render_status_rows(rows, color = True)
|
||||
# Status column has ANSI, but APR columns should not
|
||||
@@ -396,11 +408,41 @@ class TestAprRendering:
|
||||
data_lines = [line for line in output.splitlines() if "|" in line and "APR" not in line and not line.startswith("+") and not line.startswith("S")]
|
||||
for line in data_lines:
|
||||
cells = [c.strip() for c in line.split("|") if c.strip()]
|
||||
# cells[0]=id, cells[1]=title, cells[2]=status (coloured), cells[3]=apr_repost, cells[4]=apr_update
|
||||
# cells[2] should have ANSI; cells[3] and cells[4] should not
|
||||
if len(cells) >= 5:
|
||||
assert "\x1b[" not in cells[3], "APR repost cell must not be coloured"
|
||||
assert "\x1b[" not in cells[4], "APR update cell must not be coloured"
|
||||
# cells[0]=id, cells[1]=filename, cells[2]=title, cells[3]=status (coloured), cells[4]=apr_repost, cells[5]=apr_update
|
||||
# cells[3] should have ANSI; cells[4] and cells[5] should not
|
||||
if len(cells) >= 6:
|
||||
assert "\x1b[" not in cells[4], "APR repost cell must not be coloured"
|
||||
assert "\x1b[" not in cells[5], "APR update cell must not be coloured"
|
||||
|
||||
# -- render: coloured APR output -------------------------------------- #
|
||||
|
||||
def test_apr_coloured_stripped_equals_plain(self) -> None:
|
||||
"""With APR columns, strip(coloured) == plain and APR cells stay uncoloured."""
|
||||
rows = [
|
||||
StatusRow(
|
||||
title = "A", ad_id = "1", filename = "a.yaml",
|
||||
status = "published-local",
|
||||
apr_repost = "due: 9", apr_update = "not due",
|
||||
),
|
||||
]
|
||||
plain = render_status_rows(rows, color = False)
|
||||
coloured = render_status_rows(rows, color = True)
|
||||
stripped = _ANSI_RE.sub("", coloured)
|
||||
assert stripped == plain, (
|
||||
"Stripped coloured output must match plain when APR columns present"
|
||||
)
|
||||
|
||||
# APR cells are not coloured (only the status column is)
|
||||
data_lines = [
|
||||
line for line in coloured.splitlines()
|
||||
if "|" in line and not line.startswith("+") and "APR" not in line and not line.startswith("S")
|
||||
]
|
||||
for line in data_lines:
|
||||
cells = [c.strip() for c in line.split("|") if c.strip()]
|
||||
if len(cells) >= 6:
|
||||
assert "\x1b[" in cells[3], "Status cell should be coloured"
|
||||
assert "\x1b[" not in cells[4], "APR repost must not be coloured"
|
||||
assert "\x1b[" not in cells[5], "APR update must not be coloured"
|
||||
|
||||
# -- build_status_rows: APR evaluation integration --------------------- #
|
||||
# These test that evaluate_auto_price_reduction is called correctly
|
||||
|
||||
Reference in New Issue
Block a user