diff --git a/api/services/csv_parse.py b/api/services/csv_parse.py index 555607d..04f9eff 100644 --- a/api/services/csv_parse.py +++ b/api/services/csv_parse.py @@ -157,6 +157,21 @@ def sanitize_columns(headers: list[str]) -> list[str]: return out +def extra_cells(raw: list[str], width: int) -> list[str]: + """Non-empty cells beyond the header width (#82). + + A row wider than its header would otherwise lose those values silently. + Empty trailing cells (a trailing comma, common in spreadsheet exports) + carry no data and are ignored. + """ + return [c for c in raw[width:] if c.strip()] + + +def ragged_row_reason(raw: list[str], width: int) -> str: + return (f"Row has {len(raw)} cells but the header has {width}; " + f"the extra value(s) would be lost, so the row was not imported.") + + def valid_identifier(name: str) -> bool: return bool(_IDENT_RE.match(name)) diff --git a/api/services/dynamic_loader.py b/api/services/dynamic_loader.py index 2949f68..5f70390 100644 --- a/api/services/dynamic_loader.py +++ b/api/services/dynamic_loader.py @@ -22,7 +22,9 @@ from api.services.csv_parse import ( ALLOWED_TYPES, cast_value, + extra_cells, parse_csv, + ragged_row_reason, sanitize_columns, valid_identifier, ) @@ -273,6 +275,13 @@ def _do_upload( values: list = [] raw_joined: list[str] = [] failed = False + extra = extra_cells(raw, len(columns)) + if extra: + if len(row_errors) < settings.MAX_ROW_ERRORS_REPORTED: + row_errors.append({"rowNumber": row_number, "column": "(beyond header)", + "value": extra[0], "reason": ragged_row_reason(raw, len(columns))}) + failed_row_count += 1 + continue for c in range(len(columns)): cell = raw[c] if c < len(raw) else "" raw_joined.append(cell) diff --git a/api/services/te_loader.py b/api/services/te_loader.py index 8c500dc..4e36195 100644 --- a/api/services/te_loader.py +++ b/api/services/te_loader.py @@ -30,8 +30,10 @@ from api.db import Conn from api.services.csv_parse import ( cast_value, + extra_cells, parse_csv, pg_type_to_allowed_type, + ragged_row_reason, sanitize_columns, ) @@ -218,6 +220,11 @@ def _err(row_number: int, column: str, value: str, reason: str) -> None: row_number = r + 1 values: list = [] failed = False + extra = extra_cells(raw, len(columns)) + if extra: + _err(row_number, "(beyond header)", extra[0], ragged_row_reason(raw, len(columns))) + failed_row_count += 1 + continue for c, col_type in enumerate(col_types): cell = raw[c].strip() if c < len(raw) else "" if cell == "": diff --git a/tests/test_ragged_rows.py b/tests/test_ragged_rows.py new file mode 100644 index 0000000..db5bd4b --- /dev/null +++ b/tests/test_ragged_rows.py @@ -0,0 +1,70 @@ +"""Rows wider than the header are reported, never silently truncated (#82).""" + +from __future__ import annotations + +import os +import unittest +import uuid + +import pytest +from fastapi.testclient import TestClient + +from api.main import app +from api.services.csv_parse import extra_cells + +_HELP = ("Start PostgreSQL and ensure api/ can reach it (PGHOST/PGPORT/PGUSER/" + "PGPASSWORD/PGDATABASE). See scripts/start-api.ps1.") + + +@pytest.mark.unit +class ExtraCells(unittest.TestCase): + def test_non_empty_cells_beyond_the_header_are_returned(self): + self.assertEqual(extra_cells(["1", "2", "x", ""], 2), ["x"]) + + def test_trailing_empty_cells_are_not_extra(self): + self.assertEqual(extra_cells(["1", "2", "", " "], 2), []) + + def test_short_rows_have_no_extra_cells(self): + self.assertEqual(extra_cells(["1"], 2), []) + + +@pytest.mark.integration +class RaggedRowsOverTheApi(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.ctx = TestClient(app) + try: + cls.client = cls.ctx.__enter__() + except Exception as exc: # noqa: BLE001 + raise AssertionError(f"Could not start the API: {type(exc).__name__}: {exc}\n{_HELP}") from None + + @classmethod + def tearDownClass(cls): + cls.ctx.__exit__(None, None, None) + + def setUp(self): + self.tag = f"{os.getpid()}_{uuid.uuid4().hex[:8]}" + + def _upload(self, content: str, **extra) -> dict: + body = {"fileName": f"ragged_{self.tag}.csv", "content": content, **extra} + return self.client.post("/api/csv/upload", json=body).json() + + def test_dynamic_mode_reports_a_row_wider_than_the_header(self): + res = self._upload(f"a,b\n1,{self.tag}\n2,{self.tag},LOST\n") + self.assertEqual(res["status"], "ok") + self.assertEqual((res["insertedRows"], res["failedRows"]), (1, 1)) + err = res["rowErrors"][0] + self.assertEqual((err["rowNumber"], err["value"]), (2, "LOST")) + self.assertIn("header has 2", err["reason"]) + + def test_dynamic_mode_accepts_a_trailing_empty_cell(self): + res = self._upload(f"a,b\n1,{self.tag},\n") + self.assertEqual((res["status"], res["insertedRows"], res["failedRows"]), ("ok", 1, 0)) + + def test_te_mode_reports_a_row_wider_than_the_header(self): + content = (f"name,org_type,country\nRAG_{self.tag}_1,government,AU\n" + f"RAG_{self.tag}_2,government,AU,LOST\n") + res = self._upload(content, mode="te", targetTable="organisations") + self.assertEqual(res["status"], "ok") + self.assertEqual((res["insertedRows"], res["failedRows"]), (1, 1)) + self.assertEqual(res["rowErrors"][0]["value"], "LOST")